Skip to content

CI: Reference workflow file name for old branch #620

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

Merged
merged 1 commit into from
May 7, 2025

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 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

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
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