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

[CI] In jenkins.cmd_utils.Sh.tee, check for failing subprocess #16382

Merged
merged 1 commit into from
Jan 12, 2024

Conversation

Lunderberg
Copy link
Contributor

@Lunderberg Lunderberg commented Jan 10, 2024

Prior to this commit, the Sh.tee method was implemented by calling f"{cmd} | tee" in subprocess.run. While the check=True flag was used, the return code was from tee, not from the command itself. This causes failures in the command itself to be silently ignored, such as in this CI pipeline in the ci/scripts/jenkins/s3.py step.

This commit updates Sh.tee to call subprocess.Popen for cmd, tee the stdout, and check the return code. (Roughly adapted from this stackoverflow post.)

Prior to this commit, the `Sh.tee` method was implemented by calling
`f"{cmd} | tee"` in `subprocess.run`.  While the `check=True` flag was
used, the return code was from `tee`, not from the command itself.
This causes failures in the command itself to be silently ignored,
such as in [this CI
pipeline](https://ci.tlcpack.ai/blue/organizations/jenkins/tvm-i386/detail/PR-16183/37/pipeline)
in the `ci/scripts/jenkins/s3.py` step.

This commit updates `Sh.tee` to call `subprocess.Popen` for `cmd`, tee
the stdout, and check the return code.  (Roughly adapted from [this
stackoverflow post](https://stackoverflow.com/a/56484734).)
@vinx13 vinx13 merged commit ae8d398 into apache:main Jan 12, 2024
7 checks passed
@Lunderberg Lunderberg deleted the ci_check_error_code_in_sh_tee branch January 16, 2024 15:40
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.

2 participants