clj-commons
Tentacles URL use of ".../git/..." is wrong
#Summary by example:
(api-call :get "repos/%s/%s/git/commits/%s" [user repo sha] options) uses the wrong URL.
(api-call :get "repos/%s/%s/commits/%s" [user repo sha] options) uses the right URL but is broken in tentacles.
The URLs behave similarly but not identically, particularly in the presence of media type specifications.
#Details:
Tentacles uses tolerated but incorrect 'git/' embedded in many Github API URLs.
For example: https://github.com/irresponsible/tentacles/blob/master/src/tentacles/data.clj#L25
While these mostly work when used against the Github API services, they do not work identically, and they are not what Github documents. Github does not have the "git/" in their URLs.
I spent the morning trying to get the diffs on a commit with the data/commit function, passing in {:accept "application/vnd.github.diff"}. But the results were the same regardless of the media type I passed.
So I checked it with curl using the same URL generated by tentacles and got the same results, and I was stymied. Tried paging parameters, all kinds of things.
Turns out that if you remove the 'git/' from the tentacles generated URL, then you get the diffs with the above media type header (which is what I wanted and expected). At least in curl. Unfortunately if you try a tentacles api-call with the modified URL it gets some ISeq error because there seems to be some reliance on the git/ text in the url that I haven't figured out yet.
Anyway, this seems to me a rather serious and pervasive error in the tentacles library. Perhaps there's some historical basis for it, I'm new to both tentacles and the Github API, so I don't know.
Actually, it's the response processing that's hosed I think, as what comes back is not a json response when you use the alternative media types and change the url to omit "git/" so that you get the upgraded response.
The response in exec-request is just a string and so seq fails on it.
The sample diff works for my commit query purposes. Note that not only does this give different output when you use the media type specification, but the vanilla commit call also will return all kinds of additional data that it was refusing to provide when using the URL that contained the git/ substring, via the :files and :stats properties.
The sample diff below is just a test. I'm guessing every single URL in tentacles needs to have its git/ substrings removed and whatever tests there are potentially rerun/updated.
tentacles-sample-commit-fix-diff.txt
Hi, thanks for the bug report.
Sadly, tentacles is currently looking for a new maintainer as I don't write clojure any more and the other members of irresponsible have no use for it.
Is it feasible to write a test that demonstrates the existing behaviour is wrong?
That would probably help convince me to ship a fix, but I can't make any promises.
Should be easy, I'll give it a whirl.
On Fri, Oct 13, 2017 at 7:33 PM, Kent Fredric [email protected] wrote:
Is it feasible to write a test that demonstrates the existing behaviour is wrong?
That would probably help convince me to ship a fix, but I can't make any promises.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/irresponsible/tentacles/issues/7#issuecomment-336590480, or mute the thread https://github.com/notifications/unsubscribe-auth/AGMwDx6-YYv53RxgzmWMKe-Cx2J5v6nJks5sr_NFgaJpZM4P3TTV .
Attached is a simple lein run project you can use to see an example of the broken data coming back from the tentacles.data module. That appears to be the only namespace using the incorrect URLs.
It does not demonstrate the thing in my previously submitted diff that is required to relay data from alternative media types (e.g. "application/vnd.github.diff" to get full text diffs from data/commit).
The correct URL returns critical things like :files that the incorrect URL does not. It's curious as to why tentacles.data is the only namespace using the broken URLS, and where/why those URls were used, and why they sort-of-but-not-really work.
Well, this is more complicated than I thought perhaps, and I'm working on a change for it which will hopefully result in a pull request someday. If you look at the Github documentation to retrieve a commit, it uses the same broken URL as tentacles, https://developer.github.com/v3/git/commits/#get-a-commit
However that's the very URL that will fail to return all the commit attribute or work with media type headers.
If you actually retrieve a commit from Github, say with tentacles.repos/commits, it returns various urls about a commit, like the :url value, which have the more functional URL without the 'git/' substring and which will actually return to you the :files and other attributes and also respect media type specifications. But then Github's own examples of media type usage to get commit data uses the URLs I was proposing to have tentacles use, so they have contradictory documentation. (Here's their own example of fetching commit diffs with the API and media type specification, and the url does NOT have 'git/' in it. https://developer.github.com/changes/2012-12-10-Diff-and-patch-media-types/
So I'm confused, and I will ask Github about it and see if I get anywhere.
Great. They're probably busy focusing on the new GraphQL API, which tentacles has no support for.
