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

Implement API endpoint for querying connection to token network #921

Closed
LefterisJP opened this Issue Aug 23, 2017 · 9 comments

Comments

Projects
None yet
5 participants
@LefterisJP
Collaborator

LefterisJP commented Aug 23, 2017

Problem Definition

We have API endpoints to connect and to leave a token network.

Once a user is connected to a token network, a connection manager has been created for that network which manages the user's connections to channels of that network. There is currently no way for users to query if they are connected to a token network.

Solution

The solution went through some changes after discussion with colleagues and the final form can be seen here

@ulope

This comment has been minimized.

Collaborator

ulope commented Aug 23, 2017

IMO the better HTTP code for "not joined" would be 404.

Also will this endpoint really be that useful (esp. in context of the webui)? Usually I'd imagine a user would want to get a list of all his joined networks and not query for them one token at a time.

@LefterisJP

This comment has been minimized.

Collaborator

LefterisJP commented Aug 23, 2017

IMO the better HTTP code for "not joined" would be 404.

@ulope Yes I am not sure about the 204. Perhaps 404 is indeed the more appropriate return code, but the reason I did not put it there is that it's the default return code for any non-existing endpoint by the REST server.

Also will this endpoint really be that useful (esp. in context of the webui)? Usually I'd imagine a user would want to get a list of all his joined networks not query for them one token at a time.

Both would be useful I think. It's easy to add what you say by doing a:

GET /api/<version>/connection

which should return always with a 200 OK and a list of joined tokens, and an empty list for no connected tokens. By connected it's always meant that a connection manager is active.

Sounds good @ulope ?

@ulope

This comment has been minimized.

Collaborator

ulope commented Aug 23, 2017

@ulope Yes I am not sure about the 204. Perhaps 404 is indeed the more appropriate return code, but the reason I did not put it there is that it's the default return code for any non-existing endpoint by the REST server.

Good point. You convinced me. While 404 is technically correct (the best kind ;)) its not very helpful to the user and also ambiguous.

Both would be useful I think. It's easy to add what you say by doing a:

True. I worded that badly. I didn't mean to suggest that we shouldn't have the single query endpoint.

GET /api/<version>/connection

which should return always with a 200 OK and a list of joined tokens, and an empty list for no connected tokens. By connected it's always meant that a connection manager is active.

Sounds good @ulope ?

Jup :)

@czepluch

This comment has been minimized.

Collaborator

czepluch commented Sep 6, 2017

closed with #934

@czepluch czepluch closed this Sep 6, 2017

@andrevmatos

This comment has been minimized.

Collaborator

andrevmatos commented Sep 6, 2017

Actually, I'm having some issues with it. I've a channel manager with only one of the two tokens currently registered, and it was the only Join Network I've ever done, and I've channels only with this one. Despite that, GET to /api/1/connection returns both tokens in the list, and GET to /api/1/connection/<token_address> always returns 0, even if I actually deposited funds in the first token (on join). Should I open a new issue for it, or just reopen this one? I can work on that soon.

@czepluch

This comment has been minimized.

Collaborator

czepluch commented Sep 6, 2017

Let's just reopen this.

@czepluch czepluch reopened this Sep 6, 2017

@andrevmatos andrevmatos self-assigned this Sep 6, 2017

@LefterisJP LefterisJP added this to the Next minor release milestone Sep 14, 2017

@LefterisJP LefterisJP added the 1 label Sep 14, 2017

@andrevmatos

This comment has been minimized.

Collaborator

andrevmatos commented Sep 15, 2017

Just for the records, in talks with @konradkonrad and @LefterisJP , we decided to change the API compared to OP in this issue. All information requested here are provided, but in a different form, and fixes were made. Full documentation on the changes are in #1001 .

@LefterisJP

This comment has been minimized.

Collaborator

LefterisJP commented Sep 15, 2017

@andrevmatos Can you still put a quote of the actual changes here for the benefit of people linking directly from the changelog?

@andrevmatos

This comment has been minimized.

Collaborator

andrevmatos commented Sep 15, 2017

Of course. Mostly from this comment:

Instead of different API endpoints for connection manager funds, sum_deposits, "valid channels", etc, we ended up with a unified API. GET on /connection will return 200 OK statuso code, with JSON object where each key is a token address which was previously joined (assuming having open channels for that token is an indicator for a joined network token), and the values are objects like: {"funds": 100, "sum_deposits": 50, "channels": 3}.

Full documentation with examples can be seen here .

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