Skip to content

Conversation

zxiiro
Copy link
Collaborator

@zxiiro zxiiro commented Jun 18, 2024

The print statements for the get_workflow_type script is problematic because the shell script calling this script is expecting the output to only be JSON. This PR resolves this by removing all print statements to covert them to a message field in the JSON return output so that the output can continue to expect to be JSON while giving us the debug data we are looking for.

@zxiiro zxiiro requested a review from ZainRizvi June 18, 2024 16:08
@zxiiro zxiiro requested a review from a team as a code owner June 18, 2024 16:08
@pytorch-bot pytorch-bot bot added the topic: not user facing topic category label Jun 18, 2024
Copy link

pytorch-bot bot commented Jun 18, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/128969

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit 2b1940e with merge base 4817180 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@zxiiro zxiiro force-pushed the zxiiro/fix-print branch 2 times, most recently from 8b327a9 to 430acfd Compare June 18, 2024 17:59
The print statements for the get_workflow_type script is problematic
because the shell script calling this script is expecting the output
to only be JSON. This PR resolves this by removing all print
statements to covert them to a message field in the JSON return
output so that the output can continue to expect to be JSON while
giving us the debug data we are looking for.

Signed-off-by: Thanh Ha <thanh.ha@linuxfoundation.org>
@zxiiro zxiiro force-pushed the zxiiro/fix-print branch from 430acfd to 2b1940e Compare June 18, 2024 18:34
@zxiiro zxiiro requested a review from jeanschmidt June 18, 2024 19:15
if user_list[0] == "!":
print("LF Workflows are disabled for everyone. Using meta runners.")
return WORKFLOW_LABEL_META
MESSAGE = "LF Workflows are disabled for everyone. Using meta runners."
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Does this really need to use a global variable?

Copy link
Collaborator Author

@zxiiro zxiiro Jun 18, 2024

Choose a reason for hiding this comment

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

Not really but I wanted to put it above so that it's documented together with the other fields that are returned to caller and immediately viewable when reading this file.

@zxiiro
Copy link
Collaborator Author

zxiiro commented Jun 18, 2024

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jun 18, 2024
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

@github-actions github-actions bot deleted the zxiiro/fix-print branch July 19, 2024 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged open source topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants