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

Re-organize /network, /network_info, /node/ids, /primary, /primary_info #1954

Closed
letmaik opened this issue Nov 27, 2020 · 2 comments · Fixed by #2059
Closed

Re-organize /network, /network_info, /node/ids, /primary, /primary_info #1954

letmaik opened this issue Nov 27, 2020 · 2 comments · Fixed by #2059

Comments

@letmaik
Copy link
Member

letmaik commented Nov 27, 2020

The network-related endpoints seem a bit random in their structure.

As a user I would have expected a hierarchical structure like:

  • /network
    • general network info without node details, maybe just node count
    • { "service_status": "OPENING|OPEN|WAITING_FOR_RECOVERY_SHARES",
        "current_view": 123,
        "primary_id": 456
      }
  • /network/nodes
    • collection of nodes
    • the primary node has an extra field "primary": true
    • optional filtering by public host/port: ?host=abc&port=444
    • { "nodes": [ .. ] } // list of /network/nodes/{id}
  • /network/nodes/{id}
    • { "node_id": "...",
        "status": "PENDING|TRUSTED|RETIRED",
        "host": "...", // public host
        "port": "...", // public port
        "rpc_host": "...", // internal host
        "rpc_port": "...", // internal port
        "primary": true // optional
       }
  • /network/nodes/self
    • 307 Temporary Redirect
    • Location: /network/nodes/123
  • /network/nodes/primary
    • 307 Temporary Redirect
    • Location: /network/nodes/456
    • Note: This is not a redirect to the remote node's endpoint.

While doing this, I would unify the response data models for nodes and not have different ones for non-primary and primary.

NOTE: is_primary (status code based) is still needed!

@letmaik
Copy link
Member Author

letmaik commented Dec 15, 2020

@eddyashton Would the URL scheme above work? There is an overload of /network/nodes/{id} and /network/nodes/self / /network/nodes/primary which in practice would be fine but I'm not sure if CCF has extra restrictions on that.

@eddyashton
Copy link
Member

@letmaik These overloads should work correctly - in EndpointRegistry::find_endpoint(), we first look for an exact match if it exists (which will find /nodes/self and /nodes/primary), and only look for a templated match (/nodes/{id}) if we don't have an exact match. I don't think we have anything testing this currently, but it should work.

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

Successfully merging a pull request may close this issue.

2 participants