Skip to content

Add RemoteAPI implementation#282

Closed
eyakubovich wants to merge 1 commit intomoby:masterfrom
eyakubovich:api-http-client
Closed

Add RemoteAPI implementation#282
eyakubovich wants to merge 1 commit intomoby:masterfrom
eyakubovich:api-http-client

Conversation

@eyakubovich
Copy link

  • Add an implemenation of Controller and related
    interfaces that makes REST calls to the API endpoint.
  • Add a new API endpoint to get endpoint info

Signed-off-by: Eugene Yakubovich eugene.yakubovich@coreos.com

@eyakubovich
Copy link
Author

This is currently slightly outdated but I can fix it up if the general direction is fine. Looking for initial feedback.

@mavenugo
Copy link
Contributor

mavenugo commented Aug 4, 2015

@eyakubovich Apologies on the delay. Overall approach LGTM. There has been a few changes in the data-structures recently and more coming via #429. But, we can get this merged if you can rebase to the latest and appropriate tests.

@eyakubovich
Copy link
Author

I will give rebase shortly.

@aboch
Copy link
Contributor

aboch commented Aug 4, 2015

@eyakubovich Was wondering whether these new files should go inside a separate client package. At the end this is a libnetwork rest client implementation, I feel it should not reside along with the server api files.

@eyakubovich
Copy link
Author

@aboch I can put it into a separate package. I was going off that the api package is all about remote API. Kind of like standard net/http package has both client and server code.

@aboch
Copy link
Contributor

aboch commented Aug 5, 2015

Thanks @eyakubovich, net/http is a good reference for this.

* Add an implemenation of Controller and related
interfaces that makes REST calls to the API endpoint.
* Add a new API endpoint to get endpoint info

Signed-off-by: Eugene Yakubovich <eugene.yakubovich@coreos.com>
@eyakubovich
Copy link
Author

There are some methods that return ErrNotImplemented or panic. If it makes sense, I can try to add the required functionality to the remote API and then implement them.

@aboch
Copy link
Contributor

aboch commented Aug 6, 2015

Thanks @eyakubovich, I think it makes sense to complete the client api.
I am not sure about ConfigureDriver(), for sure avoid to implement LeaveAll() as it is being removed (see #365).

@GordonTheTurtle
Copy link

@eyakubovich It has been detected that this issue has not received any activity in over 6 months. Can you please let us know if it is still relevant:

  • For a bug: do you still experience the issue with the latest version?
  • For a feature request: was your request appropriately answered in a later version?

Thank you!
This issue will be automatically closed in 1 week unless it is commented on.
For more information please refer to #1926

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants