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

Trace javascript action exit code instead of user logs #290

Merged
merged 2 commits into from
Jan 21, 2020

Conversation

dakale
Copy link
Contributor

@dakale dakale commented Jan 17, 2020

Fixes #283

Consuming actions from the graph should not imply knowledge of their runtime be it javascript, container etc

We use the exit code to determine step failure/success so tracing that info out is still useful for diagnostics

@@ -122,9 +122,9 @@ public async Task RunAsync(ActionRunStage stage)
else
{
var exitCode = await step;
Trace.Info($"Node Action run completed with exit code {exitCode}");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Debug or Info? Info means we need to check runner diag log, Debug goes to user log when ACTIONS_STEP_DEBUG=true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I do think Debug makes more sense there. I went ahead and also updated it for the container actions since the same reasoning should apply there as wel

@bryanmacfarlane
Copy link
Member

debug was meant for users to debug their workflows (not action authors to debug return code). trace wasn't meant as a deeper debug. but as long as it's not console out, I'm happy. I can see both sides of the argument.

@dakale
Copy link
Contributor Author

dakale commented Jan 17, 2020

I think return code is also useful for action consumers who may have provided bad inputs or otherwise misused the action

Debug is way more accessible than trace for this imo, if we consider trace as the runners log (vs the users workflow log) which should probably only be checked if theres suspicion of a bug in the runner itself as opposed to a workflow level error

Ill merge later if everyones happy with this

@dakale dakale merged commit 746c9d9 into master Jan 21, 2020
@dakale dakale deleted the 283-node-exit-codes branch January 21, 2020 16:23
AdamOlech pushed a commit to antmicro/runner that referenced this pull request Jan 28, 2021
* Trace javascript action exit code instead of user logs

* Debug instead of trace
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Runner logs infrastructure details
3 participants