Skip to content
This repository was archived by the owner on May 6, 2020. It is now read-only.

Conversation

@sboeuf
Copy link
Contributor

@sboeuf sboeuf commented Apr 12, 2017

Even when we are not allocating some IO streams, the caller can expect to receive a valid URL.

Even when we are not allocating some IO streams, the caller can expect
to receive a valid URL.

Signed-off-by: Sebastien Boeuf <sebastien.boeuf@intel.com>
@sboeuf sboeuf force-pushed the sboeuf/always_return_url branch from 7954a9b to ad1edd3 Compare April 12, 2017 15:20
@sboeuf
Copy link
Contributor Author

sboeuf commented Apr 12, 2017

@dlespiau @sameo Let's get this merge unless you have some objections !

@coveralls
Copy link

coveralls commented Apr 12, 2017

Coverage Status

Coverage increased (+0.05%) to 70.278% when pulling ad1edd3 on sboeuf/always_return_url into 664f4fa on master.

@coveralls
Copy link

coveralls commented Apr 12, 2017

Coverage Status

Coverage increased (+0.5%) to 70.725% when pulling ad1edd3 on sboeuf/always_return_url into 664f4fa on master.

@dlespiau
Copy link
Contributor

What's the use case for that?

@sboeuf
Copy link
Contributor Author

sboeuf commented Apr 12, 2017

In virtcontainers, we decided to retrieve the URL from the registerVM(), which is performed when we start the VM. Then we store this URL so that we can check that for the same pod, we are getting all the time the same URL.
But the problem is that when we start the VM, we sometimes have no containers described in the pod, they will come later by calling CreateContainer, meaning the registerVM returns an empty URL, and all the logic explained above does not work.
@sameo can you confirm ?

@dlespiau
Copy link
Contributor

dlespiau commented Apr 12, 2017

Sounds a good enough reason to me. It wasn't really designed to cache the URL + check, but why not.

lgtm

Approved with PullApprove

@dlespiau dlespiau merged commit 57a129c into master Apr 12, 2017
@dlespiau dlespiau deleted the sboeuf/always_return_url branch April 12, 2017 17:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants