google
fix: Add `DownloadContent` to support downloads of any size
Fix https://github.com/google/go-github/issues/3810. Fix https://github.com/google/go-github/issues/2480.
My biggest concern is that we now have:
- DownloadContent
- DownloadContents
- DownloadContentsWithMeta
- GetContent
- GetContents
How is a user supposed to understand which one to choose?
Good point. I will think about it.
- DownloadContents
- DownloadContentsWithMeta
These two are existing functions. Do we want to expose DownloadContents only for use?
Do we want to expose
DownloadContentsonly for use?
The DownloadContentsWithMeta function and its references were deleted. Is that preferred?
The
DownloadContentsWithMetafunction and its references were deleted. Is that preferred?
Well, it seems we have a number of people interested in this topic based on the conversation in #2480 and #3810.
I would really like to get some feedback from the individuals who care about these endpoints before we start changing it dramatically, and the only way I can think of doing that is to start listing GitHub logins that commented, unfortunately.
cc: Please provide your feedback on how these endpoints should be handled:
- @ran-arigur
- @abatilo
- @zaataylor
- @petersondmg
- @githoober
- @JayDubyaEey
My biggest concern is that we now have:
- DownloadContent
- DownloadContents
- DownloadContentsWithMeta
- GetContent
- GetContents
How is a user supposed to understand which one to choose?
(Full disclosure: Adding DownloadContent was my suggestion, so I can't independently say whether it will make sense to other people.)
FWIW, those methods are not all on the same type. Grouping it a bit differently:
- on RepositoriesService:
- GetContents(...)
- is genuinely a bit confusing, but I don't think we should change it, because it maps pretty directly to an underlying API call [doc link] and what makes it confusing is that that API is confusing.
- returns either a single RepositoryContent or a slice of RepositoryContents, depending on whether the path is a file or a directory.
- DownloadContents(...), DownloadContentsWithMeta(...)
- convenience wrappers around GetContents(...) + DownloadContent().
- behave identically, but DownloadContentsWithMeta(...) returns more information. I assume that DownloadContents(...) was created first, and then DownloadContentsWithMeta(...) was added later because there was no backward-compatible way to have DownloadContents(...) return the additional information.
- GetContents(...)
- on RepositoryContent (as returned by RepositoriesService.GetContents(...)):
- GetContent()
- returns the file content, if already available on the RepositoryContent. handles Base64-decoding if necessary.
- doesn't handle cases where the RepositoryContent doesn't contain the file content, e.g. if the file is >1MB (in which case it returns an error) or the RepositoryContent came from listing a directory (in which case it returns the empty string even if the file is non-empty).
- DownloadContent()
- equivalent to GetContent(), except that it works even if the RepositoryContent doesn't contain the file content: in that case it fetches it using the RepositoryContent's DownloadURL.
- GetContent()
I think that, with reasonable comments on each, users will be able to find a method that does what they need, and ignore the rest.
