Repository navigation
feat: implement network node discovery (Issue #111) - #119
jennifer-bowser wants to merge 17 commits into
Conversation
jennifer-bowser
added
priority:high
korikuzma
left a comment
korikuzma
left a comment
There was a problem hiding this comment.
This is a quick review from Codex. I haven’t reviewed the PR myself yet, but I can take a look later this week.
Please address node filtering, pagination, and empty-result caching, with regression tests and documentation for the new API.
…e registry to this PR
… in the 'NodeRegistry' method signatures
There was a problem hiding this comment.
This is still Codex reviewing. Kori will perform the human review tomorrow.
Please address the remaining base-URL issue before merging. I agree with moving the intended fixed URL out of Settings into a module constant; this would address the configuration-override concern. The URL normalization thread remains open pending the fix.
korikuzma
left a comment
korikuzma
left a comment
There was a problem hiding this comment.
The earlier blocking concern is addressed. Please confirm whether the discovery endpoint accepts the newly added trailing slash with a valid bearer token.
| VLM_NETWORK_ROOT_URL: str = "https://vlm-auth.us.auth0.com/" | ||
| VLM_NETWORK_AUDIENCE_URL: str = VLM_NETWORK_ROOT_URL + "api/v2/" | ||
| VLM_NETWORK_TOKEN_REQUEST_URL: str = VLM_NETWORK_ROOT_URL + "oauth/token/" | ||
| VLM_NETWORK_NODES_REQUEST_URL: str = VLM_NETWORK_ROOT_URL + "api/v2/clients/" |
There was a problem hiding this comment.
Codex finding: Clarification question (severity unassigned); Non-blocking: Verify the discovery endpoint’s trailing slash
Moving this URL into a constant also changes /api/v2/clients to /api/v2/clients/. On vlm-auth.us.auth0.com, unauthenticated GETs return 401 without the trailing slash and 404 with it. Does the new URL succeed with a valid bearer token? I haven’t verified authenticated behavior.
To preserve the previous endpoint, the replacement would be:
| VLM_NETWORK_NODES_REQUEST_URL: str = VLM_NETWORK_ROOT_URL + "api/v2/clients/" | |
| VLM_NETWORK_NODES_REQUEST_URL: str = VLM_NETWORK_ROOT_URL + "api/v2/clients" |
closes #111