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) RepositoryContentsClient.GetArchive does not return the expected binary content #2803

Merged
merged 6 commits into from Oct 16, 2023
Merged

Conversation

Jericho
Copy link
Contributor

@Jericho Jericho commented Oct 14, 2023

Resolves #2802


Before the change?

RepositoryContentsClient.GetArchive expects the result of the API call to contain a byte[] when in fact a stream is returned. This bug is due to a change introduced here.

After the change?

RepositoryContentsClient.GetArchive invokes Connection.GetRaw rather than Connection.Get<byte[]> in order to be the binary content of the response.

Pull request checklist

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)

Does this introduce a breaking change?

  • Yes
  • No

To ensure that I was not introducing a breaking change, I created an overload of the existing GetRaw method to accept a timeout parameter. I could have added an additional parameter to the existing GetRaw method, but that would have been a breaking change. If you prefer the "additional-parameter-to-existing-method" rather than "overload-existing-method", I'll be happy to modifiy my PR.


This unit test currently fails which demonstrates the problem and it should pass when I am done with the PR
I debated adding an optional parameter for the timeout to the existing GetRaw method (which would be my personal preference) but it would be a breaking change and I doubt the Octokit team would be interested in such a change
… repo's archive content.

This ensure stream content are handled properly and solves the problem described in GitHub issue 2802
@Jericho Jericho changed the title Getarchive (fix) RepositoryContentsClient.GetArchive does not return the expected binary content Oct 14, 2023
Copy link
Member

@kfcampbell kfcampbell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm comfortable with the decision to overload here.

@kfcampbell kfcampbell merged commit 1eac831 into octokit:main Oct 16, 2023
6 checks passed
@Jericho Jericho deleted the getarchive branch October 16, 2023 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[BUG]: RepositoryContentsClient.GetArchive does not return the expected binary content
2 participants