utils/github: rewrite get_workflow_run using GraphQL #13124
Conversation
Bo98
added
the
critical
|
Review period skipped due to |
|
Let's merge this as this is really critical. I had a quick look at the code and it looks good to me (though I am not an expert so I trust you for the GraphQL part). |
|
@Bo98 tested here, looks like it failed with another issue: https://github.com/Homebrew/homebrew-core/runs/5979330787?check_suite_focus=true |
Follow up adter Homebrew#13124 I made the choice to convert the pr variable to an integer at the very end and adjust the tests. It would be maybe more consistent to work with an integer everywhere, but this needs a more careful analysis and we are in a hurry to fix the homberew-core upload CI Fixes: 2022-04-11T20:19:34.1395885Z [31mError:[0m : Variable $pr of type Int! was provided invalid value 2022-04-11T20:19:34.1398279Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/utils/github/api.rb:261:in `open_graphql' 2022-04-11T20:19:34.1399774Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/utils/github.rb:310:in `get_workflow_run' 2022-04-11T20:19:34.1403699Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/dev-cmd/pr-pull.rb:418:in `block (4 levels) in pr_pull' 2022-04-11T20:19:34.1405233Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/dev-cmd/pr-pull.rb:417:in `each' 2022-04-11T20:19:34.1406723Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/dev-cmd/pr-pull.rb:417:in `block (3 levels) in pr_pull' 2022-04-11T20:19:34.1408112Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/portable-ruby/2.6.8/lib/ruby/2.6.0/fileutils.rb:128:in `chdir' 2022-04-11T20:19:34.1408986Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/portable-ruby/2.6.8/lib/ruby/2.6.0/fileutils.rb:128:in `cd' 2022-04-11T20:19:34.1409813Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/dev-cmd/pr-pull.rb:400:in `block (2 levels) in pr_pull' 2022-04-11T20:19:34.1410671Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/vendor/portable-ruby/2.6.8/lib/ruby/2.6.0/tmpdir.rb:93:in `mktmpdir' 2022-04-11T20:19:34.1411495Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/dev-cmd/pr-pull.rb:399:in `block in pr_pull' 2022-04-11T20:19:34.1412250Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/dev-cmd/pr-pull.rb:388:in `each' 2022-04-11T20:19:34.1413056Z /home/linuxbrew/.linuxbrew/Homebrew/Library/Homebrew/dev-cmd/pr-pull.rb:388:in `pr_pull'
| workflow_payload = API.open_rest(workflow_api_url, scopes: scopes) | ||
| workflow_id_num = workflow_payload["id"] | ||
|
|
||
| query = <<~EOS |
There was a problem hiding this comment.
Would be nice to have a convenience wrapper for constructing GraphQL queries.
There was a problem hiding this comment.
I'm not sure what that would look like. It's effectively it's own language.
I've seen things like https://github.com/contentful-labs/gqli.rb.
There was a problem hiding this comment.
Yes, neither do I. I just know it's a pain to write them down. 🙂
That said, it might be possible to target a small subset of the language that we need. But that's probably better reserved for when we have a better idea of what that subset is (i.e. are using it in more places).
|
Fetching the bottle from the workflow run is broken on my tap after this change. |
|
Sorry, @shivammathur. Can you open a new issue? |
| commit_node["commit"]["checkSuites"]["nodes"].select do |suite| | ||
| suite["workflowRun"]["workflow"]["databaseId"] == workflow_id_num |
There was a problem hiding this comment.
Should probably replace this with #dig to avoid NoMethodError in #13127. Not sure if the result will make any sense upon avoiding the error though.
The previous method of searching based on PR branch was very fragile - it was already known to be broken when a PR was submitted with the
masterbranch as a source, and it has today broken entirely for some reason (likely an issue on GitHub's side).I've rewritten it to now be much more reliable. Or at least it seems to work a lot better from the very low sample size of PRs I've tested.
Although ideally this would go through the usual 24 hour review period (and maybe refactoring to avoid the
Metrics/ModuleLengthexception), I'm expediting this since our merge process over at homebrew-core is currently completely broken.