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

fix: Set id field of PullRequest, Comment and Issue to Long #200

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

sindunuragarp
Copy link

@sindunuragarp sindunuragarp commented Nov 25, 2024

We are experiencing JsonMappingException issues due to id being represented as an Integer in this client. We have observed the following data structures experiencing the issue: PullRequest, Comment, and Issue.

e.g.

Caused by: com.fasterxml.jackson.databind.JsonMappingException: Numeric value (2148718294) out of range of int (-2147483648 - 2147483647)
at [Source: REDACTED (`StreamReadFeature.INCLUDE_SOURCE_IN_LOCATION` disabled); line: 1, column: 88] (through reference chain: com.spotify.github.v3.prs.ImmutablePullRequest$Json["id"])

As of late October this year, PR ids have exceeded Integer.MAX. This is especially crippling since most Github Endpoints return an updated object as a response, which gets deserialized by the client. In the case of PullRequest, it results in all use cases related to handling PRs becoming unusable.

Official documentation states that PullRequest id is int64. This is probably the case for all entity ids as well.
https://docs.github.com/en/rest/pulls/pulls?apiVersion=2022-11-28#get-a-pull-request

Note: This PR extends from #180 which is older and has broken tests.


This PR is technically not backwards compatible (since it changes data types), but without the fix, the client is unusable for all use cases related to recent Pull Requests.

@sindunuragarp sindunuragarp marked this pull request as ready for review November 25, 2024 20:06
@sindunuragarp
Copy link
Author

Tagging @ebk45 and @Abhi347 for review 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants