Add support for downloading zip and tar archives of repositories. #967
Conversation
| Item 2 will require an update to the test framework to fix, but it should be pretty easy - adding another mock server to the list and then recording. You can do that as part of this PR if you like. Looks like there is another PR that needs to do something similar. As an alternative, you could record the interaction and then manually create a payload that points to the |
| } | ||
| | ||
| private void downloadArchive(String type, Optional<String> ref, StreamConsumer sink) throws IOException { | ||
| requireNonNull(sink, "Sink must not be null"); |
There was a problem hiding this comment.
You can use spotbugs annotation for this.
There was a problem hiding this comment.
the annotation doesn't actually enforce it at run time though as far as I know?
they provide hints for IDE and static analysis tools
| * @throws IOException | ||
| * The IO exception. | ||
| */ | ||
| public void zipball(StreamConsumer sink, String ref) throws IOException { |
There was a problem hiding this comment.
Ah, I see why you did this. This is the endpoint you're hitting, but maybe we should use a more Java-like name for the methods? I'm interested to know your opinion.
| public void zipball(StreamConsumer sink, String ref) throws IOException { | |
| public void downloadZip(StreamConsumer sink, String ref) throws IOException { |
bitwiseman left a comment
bitwiseman left a comment There was a problem hiding this comment.
I updated this with new library-wide functional interfaces. Also fixed the codeload and redirect issue.
The main question I have is whether it is better to have the public readZip and readTar methods take a Consumer or a Function. This matters because implementing one now and adding the other later may result in compilation errors - the compiler may not be able to distinguish between the two types for some lambda inputs.
timja left a comment
timja left a comment There was a problem hiding this comment.
not sure either way on whether consumer of function is better
| } | ||
| | ||
| private void downloadArchive(String type, Optional<String> ref, StreamConsumer sink) throws IOException { | ||
| requireNonNull(sink, "Sink must not be null"); |
There was a problem hiding this comment.
the annotation doesn't actually enforce it at run time though as far as I know?
they provide hints for IDE and static analysis tools
| @timja Thanks for taking the time to review. |
bitwiseman force-pushed the download-repository-archives branch from 2367e42 to 936ab49 Compare 3 participants
👋, found myself needing to download zip/tar archives of repository content which wasn't supported so mucked about with getting that in place.
Now, there's a few things to note here which I'm not sure if/how to tackle:
302redirect to the actual archive which is hosted athttps://codeload.github.com/<owner>/<repo>/legacy.<tar.gz|zip>/<ref>but the redirect doesn't seem to be captured properly by wiremock and the download itself isn't captured at all even when adding manual logic to follow the redirectRequester.clientin order to be able to stream the content directly into aStreamConsumersinceRequester.fetchStream()reads everything into memory which didn't seem ideal for dealing with entire repository archives