Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

posthook don't log script errors #262

Open
davidcoutadeur opened this issue Jan 19, 2024 · 3 comments · Fixed by #263
Open

posthook don't log script errors #262

davidcoutadeur opened this issue Jan 19, 2024 · 3 comments · Fixed by #263
Assignees
Milestone

Comments

@davidcoutadeur
Copy link
Contributor

Currently, posthook don't log script errors (see https://lsc-project.org/documentation/latest/syncrules.html#hooks-lsc-tasks-task-propertiesbasedsyncoptions-hooks)

It could be nice to log script stderr (and maybe also stdout?)

We need to check if this has an impact on performances

@coudot
Copy link
Member

coudot commented Jul 9, 2024

I have a suggestion: have different levels for STDOUT and STDERR.

Seems easy to add an option in printHookOutput to set the logback level. I would choose DEBUG or INFO for STDOUR and ERROR for STDERR, instead of having WARN for both.

What do you think @davidcoutadeur ?

@coudot coudot reopened this Jul 9, 2024
@davidcoutadeur
Copy link
Contributor Author

Why not, it does not seem too difficult indeed.

Note: until know, the script were not supposed to write things to stdout/stderr, we should update the documentation:

Hook scripts are not supposed to write anything to stdout or stderr. If so, LSC will display theses messages as log warnings.

@davidcoutadeur
Copy link
Contributor Author

davidcoutadeur commented Jul 12, 2024

Done in commit 40d80fe

I have set level INFO for stdout and ERROR for stderr

I am updating the corresponding documentation right now.

davidcoutadeur pushed a commit to lsc-project/documentation that referenced this issue Jul 12, 2024
See also corresponding issue on main lsc project:
lsc-project/lsc#262
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants