Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: Feature parity with Rust launcher #2
base: master
Are you sure you want to change the base?
feat: Feature parity with Rust launcher #2
Changes from 1 commit
3a2f8ea
eea7e1b
f91866f
aa7f09f
03552c1
f82a95f
0599771
7b50b60
357c2ed
0b6484d
4227665
2595006
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logger writes to stderr by default, which is not what we would want here. Also could we enable these logs only if certain env var is set, similar to
RUST_LOG
in the rust launcherThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we want to allow hiding the launcher logs by default? n8n main, worker, webhook, etc. all show at least info logs by default. As a user I'd like to always have a few informative lines to know the runner is working as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would consider most of these logs to be debug logs, like for example the loading of the config here. Sure we can keep one or two to know the runner is working, (tho it can also be interpreted from the log from n8n that runner has connected), but we don't need all of these. This would be in quite contrast compared to what we log in the n8n app itself, which is very minimal
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, let's do this division of info/debug logs as a follow up 👍🏻