-
Notifications
You must be signed in to change notification settings - Fork 15
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
[Suggestion] allow to expose the commit id through other ways #34
Comments
That seems reasonable - Prout could read a
You can already do this! Prout doesn't require that the commit id is in an HTML comment, or even in any particular format other than being an isolated 40-char hex string - so if you want to keep your API JSON only, just return the hex id as part of a JSON manifest/metadata endpoint, just point to that endpoint your |
Great that it already works for the api endpoint. It would be great to improve the readme about the way the commit id should be exposed. |
Does it need to be 40-char hex ? since commit ids are completely identifiable within repo by shorter version like
Again does the endpoint has to return in JSON ?? I am trying to leverage |
Prout expects a 40-char hex: https://github.com/guardian/prout/blob/25e4e5a60/app/lib/CheckpointSnapshot.scala#L42 In principle, you could go shorter, but for all use-cases I'm aware of it's not really any trouble to specify the full commit id (is there a reason why you can't?) - and resolving shorter ids just exposes one possible avenue for ambiguity. |
Yeah I see what you mean by one more avenue of ambiguity . I was only trying to leverage what is given currently by our deployment tool. Had it worked, we would have a poor man's API endpoint under http://ourwebsite.uri/revision.txt spitting out short version 7-char HEX of git commit For the sake of testing, we configured http://ourwebsite.uri/revision.txt to return full 40-char hex in plaintext format. We are getting this in our web-server logs
is that expected output ? does the endpoint has to return in JSON ?? like in your example https://members-data-api.theguardian.com/healthcheck |
No, so long as the response body contains a 40-char hex, it can be text, html, or json, whatever. For instance, in the html of https://membership.theguardian.com/ , there's this HTML comment:
|
👍 Super ! Sorry following query might deviate from discussion happening under this thread. What happens when http://ourwebsite.uri/revision.txt returned commit point that has no .prout.json configured; let's say that is
Is 204 status code correct in above scenario ? .prout.json {
"checkpoints": {
"PROD": { "url": "http://ourwebsite.uri/revision.txt", "overdue": "10M" }
}
} Repository is correctly configured already to deliver web-hook to on-premise hosted prout. |
Using a comment in the HTML works only for HTML websites. This does not play well with other kind of projects (APIs for instance).
Would it be possible to expose the commit id in other ways ? I see 2 potential ways:
The text was updated successfully, but these errors were encountered: