Skip to content

Conversation

@cryos
Copy link
Collaborator

@cryos cryos commented May 7, 2025

Description

Switch to use the workflow file name as the name is getting missed in the merged code in main. Verified the behavior locally too. This is a follow up from #555 addressing an issue seen after merging.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Switch to use the workflow file name as the name is getting missed in
the merged code in main. Verified the behavior locally too.
@copy-pr-bot
Copy link
Contributor

copy-pr-bot bot commented May 7, 2025

Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@cryos
Copy link
Collaborator Author

cryos commented May 7, 2025

/ok to test

@github-actions

This comment has been minimized.

@cryos cryos requested a review from leofang May 7, 2025 15:34
@cryos
Copy link
Collaborator Author

cryos commented May 7, 2025

@leofang see failures after merge where the workflow name is not found. In general I have used workflow file names and they have been reliable.

@leofang leofang added bug Something isn't working P0 High priority - Must do! CI/CD CI/CD infrastructure labels May 7, 2025
@leofang leofang added this to the cuda-python parking lot milestone May 7, 2025
Copy link
Member

@leofang leofang left a comment

Choose a reason for hiding this comment

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

Thanks, Marcus! Yes, I noticed the main failed too. So instead of backporting the CI name change, we just fix it by referring to the workflow filename. (I did not know this works!) The only catch is that when the current main branch becomes the backport branch in the future, we'll need to change it to ci.yaml.

@leofang
Copy link
Member

leofang commented May 7, 2025

One question: Why didn't we observe this failure in #555?

@cryos
Copy link
Collaborator Author

cryos commented May 7, 2025

One question: Why didn't we observe this failure in #555?

Not clear and that is frustrating! I guess we hit a corner case, I will merge this and see if it is more reliable.

@cryos
Copy link
Collaborator Author

cryos commented May 7, 2025

Thanks, Marcus! Yes, I noticed the main failed too. So instead of backporting the CI name change, we just fix it by referring to the workflow filename. (I did not know this works!) The only catch is that when the current main branch becomes the backport branch in the future, we'll need to change it to ci.yaml.

Yes, and I think I will have factored out some of this common code into helper scripts better by that point too.

@cryos cryos merged commit ee6b92e into NVIDIA:main May 7, 2025
56 checks passed
@github-actions
Copy link

github-actions bot commented May 7, 2025

Doc Preview CI
Preview removed because the pull request was closed or merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working CI/CD CI/CD infrastructure P0 High priority - Must do!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants