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

gateway: /version tests, add CurrentCommit #2001

Merged
merged 4 commits into from
Dec 1, 2015
Merged

gateway: /version tests, add CurrentCommit #2001

merged 4 commits into from
Dec 1, 2015

Conversation

ghost
Copy link

@ghost ghost commented Nov 24, 2015

I'm not 100% sure this is a good idea, but I though I'd put it up for discussion. Chatted about this briefly with @harlantwood, and think it's useful.

Need to build with make build so that repo/config.CurrentCommit gets set, otherwise it'll just be an empty string.

@ghost ghost added the topic/gateway Topic gateway label Nov 24, 2015
@jbenet jbenet added the status/in-progress In progress label Nov 24, 2015
@ghost ghost added need/community-input Needs input from the wider community RFCR labels Nov 24, 2015
@ghost
Copy link
Author

ghost commented Nov 24, 2015

Some of these CircleCI failures are horrible :/

not ok 41 - 'ipfs add -r' output looks good

None look related to this PR. Damn, we really need to sort this out.

@whyrusleeping
Copy link
Member

yeah, the random CI failures have gotten steadily worse in the past few weeks... :/

on topic: I think adding this to the version endpoint should be fine

@ghost
Copy link
Author

ghost commented Nov 25, 2015

I have a better idea, let's remove :8080/version and enable :5001/api/v0/version instead.

@ghost
Copy link
Author

ghost commented Nov 25, 2015

Output is nice and machine readable, plus we can kill some code

{
  "Version": "0.3.10-dev",
  "Commit": "20b06a4",
  "Repo": "2"
}

Lars Gierth added 2 commits November 25, 2015 02:51
License: MIT
Signed-off-by: Lars Gierth <larsg@systemli.org>
License: MIT
Signed-off-by: Lars Gierth <larsg@systemli.org>
@whyrusleeping
Copy link
Member

mmm, the version endpoint on the gateway is something i wanted for ease of debugging purposes. i'd prefer to keep it there

@harlantwood
Copy link
Contributor

Same, I'd like to get the version running on IPFS.io ....

@ghost
Copy link
Author

ghost commented Nov 25, 2015

What I mean is, the API is accessible via the Gateway anyhow, so it's :8080/version vs. :8080/api/v0/version, while the latter is even machine readable.

@ghost ghost mentioned this pull request Nov 26, 2015
42 tasks
@ghost
Copy link
Author

ghost commented Nov 26, 2015

I'll just do both :8080/version and :8080/api/v0/version for now

Lars Gierth added 2 commits November 26, 2015 02:46
License: MIT
Signed-off-by: Lars Gierth <larsg@systemli.org>
License: MIT
Signed-off-by: Lars Gierth <larsg@systemli.org>
@harlantwood
Copy link
Contributor

harlantwood commented Nov 26, 2015 via email

@whyrusleeping
Copy link
Member

This LGTM

@whyrusleeping whyrusleeping removed the status/in-progress In progress label Nov 28, 2015
@ghost ghost added RFM and removed RFCR labels Nov 29, 2015
@ghost ghost mentioned this pull request Nov 30, 2015
14 tasks
@jbenet
Copy link
Member

jbenet commented Dec 1, 2015

yeah i want to keep both too.

jbenet added a commit that referenced this pull request Dec 1, 2015
gateway: /version tests, add CurrentCommit
@jbenet jbenet merged commit 7d0d1af into master Dec 1, 2015
@jbenet
Copy link
Member

jbenet commented Dec 1, 2015

LGTM

@jbenet jbenet deleted the version-commit branch December 1, 2015 15:30
@rht
Copy link
Contributor

rht commented Dec 16, 2015

!!

@lgierth as of this commit, 9beebc9, the test file needs to be updated too.

In particular, https://github.com/ipfs/go-ipfs/blob/9beebc9779a765a567b20e5ac9915467685802e9/test/sharness/t0110-gateway.sh#L114, version needs to be removed from the sanitization list. Did this particular test pass?

@ghost
Copy link
Author

ghost commented Dec 16, 2015

It doesn't look like it was even run... :/

@jbenet
Copy link
Member

jbenet commented Dec 16, 2015

hmmm is that not running then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/community-input Needs input from the wider community RFM topic/gateway Topic gateway
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants