Skip to content

Replace all -F usage with -f #350

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 7 commits into from
May 15, 2025

Conversation

jarmak-nv
Copy link
Contributor

This pull request updates multiple GitHub Actions workflows to standardize the usage of the gh api graphql command by replacing the -F flag with the -f flag for all GraphQL variable inputs. This change ensures consistency and aligns with best practices for the gh CLI.

From GitHub

Using variables
In all of the following examples, you can use variables to simplify your scripts. Use -F to pass a variable that is a number, Boolean, or null. Use -f for other variables. For example,

my_org="octo-org"
my_num=5
gh api graphql -f query='
  query($organization: String! $number: Int!){
    organization(login: $organization){
      projectV2(number: $number) {
        id
      }
    }
  }' -f organization=$my_org -F number=$my_num

For more information, see Forming calls with GraphQL.

I'm unsure if this is a recent change, but we hit the failure when attempting to convert an integer into a string. GitHub may silently fix string-likes.

targeting python3.13 as cuDF found the failure; can merge to main after confirmation of resolution.

@jarmak-nv jarmak-nv requested a review from a team as a code owner May 14, 2025 18:42
@jarmak-nv jarmak-nv requested review from bdice and removed request for a team May 14, 2025 18:42
@jarmak-nv jarmak-nv added bug Something isn't working non-breaking Introduces a non-breaking change labels May 14, 2025
@jameslamb jameslamb self-requested a review May 14, 2025 18:47
@jameslamb jameslamb changed the base branch from python-3.13 to branch-25.06 May 14, 2025 18:48
@jameslamb
Copy link
Member

@jarmak-nv before we merge... I noticed this was targeted at the python-3.13 branch instead of branch-25.06... I suspect that's a mistake.

It does looks like there are some lingering references to the @python-3.13 branch (GitHub Search), but the PR for it (#268) has already been merged. I don't think we're planning to merge any new commits from the python-3.13 back into branch-25.06 at this point.

I've retargeted this at branch-25.06. I tried to do a rebase and force push, but I can't do that because this came from your fork. Could you please:

  1. rebase this on the latest branch-25.06
  2. force-push those changes to this branch

And then I think the deployment story should be:

  1. merge this into branch-25.06
  2. merge branch-25.06 into Add CUDA 12.9.0 #339
  3. these changes will be out on all the repos branch-25.06 branches once the CUDA 12.9 migration is done and they're switched back to branch-25.06 (cc @gforsyth @bdice )

@jameslamb jameslamb self-requested a review May 14, 2025 18:58
@vyasr
Copy link
Contributor

vyasr commented May 14, 2025

I think there's some confusion in part because it's going to be a bit before all the repos are using the same branch of shared-workflows anyway at this point, and I'm the one asking Ben for these workflow fixes that are blocking cudf's board automations (I don't think any other repo is using them, or at least not to the same extent) but cudf is lagging in some of the migrations due to e.g. the CUDA 12.9 compiler issues. Let's stick with James's suggestion for the merge plan. Worst case we need to merge down branch-25.06 into one of the migration branch sooner for more testing.

@jarmak-nv jarmak-nv force-pushed the f-fix-and-permission-issue branch from 861fe12 to 27ae5ad Compare May 14, 2025 19:45
@jarmak-nv
Copy link
Contributor Author

Should be g2g

Will note cuML also uses these, currently pinned to branch-25.06.

@jarmak-nv
Copy link
Contributor Author

@jameslamb - when you're on, any thoughts on this CI failure?

Lint GitHub Actions workflow files.......................................Failed
- hook id: actionlint-docker
- exit code: 1

Executable `docker` not found

Pre-commit linting passes on my end, but maybe I broke something? 😅

@gforsyth
Copy link
Contributor

@jarmak-nv -- this looks like the recently added actionlint-docker check doesn't work on pre-commit.ci -- it's also broken on main, so I don't think you broke anything.

@bdice
Copy link
Contributor

bdice commented May 15, 2025

Sorry! I didn’t realize that hook required Docker. I just copied it from rapidsai/shared-actions. Perhaps we need to disable it in CI, or run in our own infrastructure.

@gforsyth
Copy link
Contributor

I'll push up a commit disabling it for now -- I think we can run it on our own infrastructure but I need to familiarize myself with how we run workflows within shared-workflows

@gforsyth
Copy link
Contributor

Ok, disabled it for CI only, so it'll still run locally. Going to merge this now

@gforsyth gforsyth merged commit 0e76dc4 into rapidsai:branch-25.06 May 15, 2025
2 checks passed
@gforsyth
Copy link
Contributor

Thanks @jarmak-nv !

@jarmak-nv
Copy link
Contributor Author

Thanks all! Appreciate the support 🚀

@jarmak-nv jarmak-nv deleted the f-fix-and-permission-issue branch May 15, 2025 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants