-
Notifications
You must be signed in to change notification settings - Fork 65
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
Multi-gitter doesnt attempt to open pull requests for branches that already exist #206
Comments
This is expected behavior since otherwise we could open pull requests for changes that were not intended to be made if branch names are re-used. I think this is closely related to #205, so let's keep the discussion in one single place. PS. I will add some kind of warning about rate-limiting when using concurrency. How many concurrent operations did you do? |
I had concurrency set to one, but was running it over 200+ repos. GitHub
docs recommend a one second delay between pull requests, I wonder if we
should implement this.
…On Thu., Oct. 28, 2021, 1:45 a.m. Johan Lindell, ***@***.***> wrote:
This is expected behavior since otherwise we could open pull requests for
changes that were not intended to be made if branch names are re-used. I
think this is closely related to #205
<#205>, so let's keep the
discussion in one single place.
PS. I will add some kind of warning about rate-limiting when using
concurrency. How many concurrent operations did you do?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#206 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AR5TNSRRMYBSTI7JGPOMUU3UJDWQZANCNFSM5G3SM37Q>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
If there should not be more than one pr per second. It sounds reasonable to implement that. Do you have a link which states that, I can't seem to find it. |
Here is the doc:
https://docs.github.com/en/rest/guides/best-practices-for-integrators#dealing-with-secondary-rate-limits
…On Thu., Oct. 28, 2021, 11:27 a.m. Johan Lindell, ***@***.***> wrote:
If there should not be more than one pr per second. It sounds reasonable
to implement that. Do you have a link which states that, I can't seem to
find it.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#206 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AR5TNSXWKEPWGO2QXD3GVNDUJF2WXANCNFSM5G3SM37Q>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Thanks 🙂 I created a PR that makes sure that we don't hit the secondary rate limit #207 |
Would the expectation then be that the least likely chance of hitting this
would be setting concurrent executions to one?
For example, setting it to five causes me to hit the rate limit still.
…On Thu., Oct. 28, 2021, 12:23 p.m. Johan Lindell, ***@***.***> wrote:
Thanks 🙂
I created a PR that makes sure that we don't hit the secondary rate limit
#207 <#207>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#206 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AR5TNSWBZ2BN2TZKMO2NDY3UJGBJVANCNFSM5G3SM37Q>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Do you know if you hit the primary or secondary rate limiting? Could you please post |
I hit the secondary. Tracelogs I can post later.
…On Thu., Oct. 28, 2021, 3:28 p.m. Johan Lindell, ***@***.***> wrote:
Do you know if you hit the primary or secondary rate limiting? Could you
please post --log-level=trace logs.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#206 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AR5TNSTD5BRSFVUVTEVQBJ3UJGW77ANCNFSM5G3SM37Q>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
Describe the bug
When running the run command, if you lose connection to github between when the branch is created and when the pull request is opened then you will be unable to open a pull request with the created branch using multi-gitter. I've also noticed if you hit the pull request rate limiting when concurrency is enabled then multi-gitter will continue to create branches but won't re-attempt to open any pull requests. Re-running the tool just tells you the branch already exists, but I would expect if the branch already exists but there isn't a pull request that it would try to create a pull request based on that branch.
Expected behavior
Branches that exist but don't have an associated pull request attempt to open a pull request
The text was updated successfully, but these errors were encountered: