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

Modify run.sh to use redirects rather than screen #93

Merged
merged 8 commits into from
Oct 25, 2022
Merged

Conversation

solvaholic
Copy link
Owner

@solvaholic solvaholic commented Oct 11, 2022

Modify run.sh to use redirects, rather than screen, to create plan and log files.

Description

Currently run.sh relies on screen to write command exit status in its output file. This pull request replaces that with shell redirects.

Motivation and Context

screen on a given Actions runner may not write command exit status in its output file.

Fixes #92

How Has This Been Tested?

Tested the redirects manually, locally, running octodns sync in shells of Docker containers; Pushed the code to issue92 branch and ran it in triggered workflows using solvaholic/octodns-sync@issue92.

See also #92 (comment)

To-Do List

to create plan and log files. /cc #92
@solvaholic solvaholic added the bug Something isn't working label Oct 11, 2022
@solvaholic solvaholic self-assigned this Oct 11, 2022
@solvaholic
Copy link
Owner Author

Timely, as this project uses set-output and already needs work on its outputs:
https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

@solvaholic
Copy link
Owner Author

From #92 (comment):

Somewhat related .. while looking for context I see this project used to use tee (#23) and after moving to screen (#24) it was still/again possible for this action to silently fail (#40).

@solvaholic solvaholic marked this pull request as ready for review October 18, 2022 00:15
@solvaholic
Copy link
Owner Author

I ran 3a27125 successfully in solvahol/dns. I'd like to also confirm the outputs work, and see how the Action behaves when octodns sync fails.

@laszlof if you're still running @issue92, or would like to check it again, please let me know how it goes.

@laszlof
Copy link

laszlof commented Oct 20, 2022

@solvaholic I'll give this a shot in the coming days. I've been out of town this week. Thanks for the fixes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Script exiting with non-zero exist status even if successful
2 participants