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

Move api version check to daemon #2003

Closed
wants to merge 2 commits into from
Closed

Move api version check to daemon #2003

wants to merge 2 commits into from

Conversation

rht
Copy link
Contributor

@rht rht commented Nov 24, 2015

closes #1990

@jbenet jbenet added the status/in-progress In progress label Nov 24, 2015
@rht
Copy link
Contributor Author

rht commented Nov 24, 2015

Doesn't solve the closenotify chan problem, however.

@whyrusleeping
Copy link
Member

Awesome! looks like it breaks tests though

@rht
Copy link
Contributor Author

rht commented Nov 24, 2015

Saw it, possibly because now that the daemon demands the api header. Make it optional? (or make it necessary)

@rht rht force-pushed the daemon-version-check branch 2 times, most recently from 72e090e to 977aed3 Compare November 24, 2015 07:55
@rht
Copy link
Contributor Author

rht commented Nov 24, 2015

I picked the optional, since it was the default behavior previously.

License: MIT
Signed-off-by: rht <rhtbot@gmail.com>
@rht
Copy link
Contributor Author

rht commented Nov 24, 2015

Not sure how to fix the pollEndpoint errs.

@whyrusleeping
Copy link
Member

those errors look like its the daemon failing to start.

License: MIT
Signed-off-by: rht <rhtbot@gmail.com>
@jbenet
Copy link
Member

jbenet commented Dec 1, 2015

👍 on this PR. Optional sounds good to me, too.

sharness tests still failing.


// make sure the api is actually running.
// this is slow, as it might mean an RTT to a remote server.
// TODO: optimize some way
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, there's a problem here. part of the goal is to check that the endpoint is available, which may be a case of a stale $repo/api file (eg ipfs daemon, then kill -9 $pid). we could "check" by trying to use it, but im no longer sure how the flow will work here. I recall it being tricky to get right + cover all the bases.

@rht
Copy link
Contributor Author

rht commented Dec 16, 2015

(moved to 0.3.11 #2037)

@rht rht closed this Dec 16, 2015
@jbenet jbenet removed the status/in-progress In progress label Dec 16, 2015
@Kubuxu Kubuxu deleted the daemon-version-check branch February 27, 2017 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants