.ci/merge-fixes.sh: GraphQL: API rate limit exceeded
#36685
Labels
Comments
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Dec 7, 2023
…customizable by repository variable
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->
Instead of using `gh pr checkout`, we obtain the CI fixes via their
patch URLs.
- This is faster because typically we do not have to unshallow the repo
to apply the patches (seconds instead of ~2 minutes)
- Fewer surprises when applied to a PR based on an older release
- Conjecturally uses fewer API queries, helping avoid sagemath#36685
When a repository variable `SAGE_CI_FIXES_FROM_REPOSITORIES` is set in a
fork, it is used instead of the hardcoded sagemath/sage as the source(s)
of the CI fixes; this gives better control in decentralized development.
When set to "none", this also makes it possible to see the "ground
truth", addressing a concern raised in
sagemath#36349 (comment). See
comments at the top of `.ci/merge-fixes.sh` for details.
Also improving the display of details of the merged PRs, as requested by
@tornaria in
sagemath#36696 (comment)
Example runs:
- https://github.com/sagemath/sage/actions/runs/6854931832/job/186389018
59?pr=36686 (here it recovers gracefully from failed merges due to the
10.2.rc1/10.2.rc2 tagging confusions)
- https://github.com/mkoeppe/sage/actions/runs/6855410957/job/1864038039
8#step:3:8 (I have set `SAGE_CI_FIXES_FROM_REPOSITORIES=none` in my fork
-- for testing this PR)
<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->
### 📝 Checklist
<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->
- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.
### ⌛ Dependencies
<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
URL: sagemath#36686
Reported by: Matthias Köppe
Reviewer(s): Gonzalo Tornaría, Kwankyu Lee, Matthias Köppe
vbraun
pushed a commit
to vbraun/sage
that referenced
this issue
Dec 9, 2023
…customizable by repository variable
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->
Instead of using `gh pr checkout`, we obtain the CI fixes via their
patch URLs.
- This is faster because typically we do not have to unshallow the repo
to apply the patches (seconds instead of ~2 minutes)
- Fewer surprises when applied to a PR based on an older release
- Conjecturally uses fewer API queries, helping avoid sagemath#36685
When a repository variable `SAGE_CI_FIXES_FROM_REPOSITORIES` is set in a
fork, it is used instead of the hardcoded sagemath/sage as the source(s)
of the CI fixes; this gives better control in decentralized development.
When set to "none", this also makes it possible to see the "ground
truth", addressing a concern raised in
sagemath#36349 (comment). See
comments at the top of `.ci/merge-fixes.sh` for details.
Also improving the display of details of the merged PRs, as requested by
@tornaria in
sagemath#36696 (comment)
Example runs:
- https://github.com/sagemath/sage/actions/runs/6854931832/job/186389018
59?pr=36686 (here it recovers gracefully from failed merges due to the
10.2.rc1/10.2.rc2 tagging confusions)
- https://github.com/mkoeppe/sage/actions/runs/6855410957/job/1864038039
8#step:3:8 (I have set `SAGE_CI_FIXES_FROM_REPOSITORIES=none` in my fork
-- for testing this PR)
<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->
### 📝 Checklist
<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->
- [x] The title is concise, informative, and self-explanatory.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.
### ⌛ Dependencies
<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
URL: sagemath#36686
Reported by: Matthias Köppe
Reviewer(s): Gonzalo Tornaría, Kwankyu Lee, Matthias Köppe
|
I think this has been fixed by #36686 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
As observed in #36372 (comment)
The text was updated successfully, but these errors were encountered: