Skip to content
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

GitHub IDs have exceeded Int32.MaxValue; Octokit uses int #2351

Closed
SeanKilleen opened this issue Aug 23, 2021 · 9 comments · May be fixed by #2962
Closed

GitHub IDs have exceeded Int32.MaxValue; Octokit uses int #2351

SeanKilleen opened this issue Aug 23, 2021 · 9 comments · May be fixed by #2962
Labels
Status: Stale Used by stalebot to clean house Type: Bug Something isn't working as documented

Comments

@SeanKilleen
Copy link
Contributor

Spotted in this tweet from @patriksvensson: https://twitter.com/firstdrafthell/status/1429738272574558210?s=19

I'll look into this ASAP but don't mind anyone else doing the same.

@shiftkey
Copy link
Member

An example PR where we've had to fix this in the past: #1703

Happy to push out a release asap about it if you have a fix.

@shiftkey
Copy link
Member

@patriksvensson got a sample script that helps us to know where it's overflowing?

@SeanKilleen
Copy link
Contributor Author

For those following along, I am using the following regex search in VS to attempt to find int Ids and knocking them out:

int [^=]*Id

@SeanKilleen
Copy link
Contributor Author

@shiftkey I'm not quite sure how the songs are chosen for releases, but this one seems like "For the Long-est Time" would be appropriate.

@shiftkey
Copy link
Member

Release titles are chosen mostly at random. I endorse this name for the one that fixes this.

@patriksvensson
Copy link
Contributor

@shiftkey I don't have a sample script, but encountered this when retrieving notifications.

@SeanKilleen
Copy link
Contributor Author

@patriksvensson question as I go through this:

You mention that you were having issues when retrieving notifications. It looks like, in a strange turn of events, the API is actually returning the notification ID as a string (and Octokit .NET is complying with that). Thread IDs are integers, but Octokit is currently compliant with that as well.

Are you by any chance casting it to an integer on your end?

The rest of this is still worth addressing, but I wanted to point out that it may not resolve your particular issue.

@nwf
Copy link

nwf commented Jan 17, 2022

Is there any progress on resolving this? #2352 appears stalled, but, sadly, this is stymieing any use of octokit.net for handling workflow_job events, because the id field therein is expressed as a JSON number, not a string, and they are far above 2**32 at this point.

JamieMagee added a commit to octokit/webhooks.net that referenced this issue Jan 24, 2022
Migrates all integer types from `int` (32-bit) to `long` (64-bit).

Could I be a bit more directed with these changes? Probably. But the it's simpler and easier to migrate all `int`s to `long`s rather than debate how large a specific ID is going to be.

Closes #30

References:
  - #30
  - octokit/octokit.net#2351
  - octokit/octokit.net#2352
  - https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/integral-numeric-types
@nickfloyd nickfloyd added Type: Bug Something isn't working as documented and removed category: bug labels Oct 26, 2022
@github-actions
Copy link

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!

@github-actions github-actions bot added the Status: Stale Used by stalebot to clean house label Jul 24, 2023
@github-actions github-actions bot closed this as completed Aug 1, 2023
kzu added a commit to kzu/octokit.net that referenced this issue Aug 27, 2024
According to the documentation at https://docs.github.com/en/rest/issues/issues.

Fixes octokit#2351 for issue ids.
@kzu kzu mentioned this issue Aug 27, 2024
2 tasks
kzu added a commit to kzu/octokit.net that referenced this issue Aug 27, 2024
According to the documentation at https://docs.github.com/en/rest/issues/issues.

Fixes octokit#2351 for issue ids.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Stale Used by stalebot to clean house Type: Bug Something isn't working as documented
Projects
None yet
6 participants