Skip to content

[transports] Evaluate the discovery HTTP server: Issue #77 - #104

Open
Shreyavemana wants to merge 2 commits into
nitrocloudofficial:developfrom
Shreyavemana:docs/77-discovery-server-evaluation
Open

Shreyavemana wants to merge 2 commits into
nitrocloudofficial:developfrom
Shreyavemana:docs/77-discovery-server-evaluation

Conversation

@Shreyavemana

Copy link
Copy Markdown
Collaborator

Summary

Investigates the TypeScript DiscoveryHttpServer and compares it with the Python SDK's existing discovery and port-handling mechanisms.

Closes #77.

Changes Made

  • Added docs/investigations/discovery-http-server.md.
  • Identified TypeScript consumers and documented how the server is used.
  • Documented Python discovery-server startup, CLI port resolution, and port-retry behavior with code references.
  • Included a feature comparison and a written conclusion on whether further implementation is needed.

Tests

  • No automated tests added; this is a documentation-only change.
  • Runtime behavior was verified where applicable.

@AadarshM07
AadarshM07 requested a review from ROHIT-JR November 2, 2026 12:23

@ROHIT-JR ROHIT-JR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes: this is still the unfilled template, so the acceptance criteria on #77 aren't met yet.

  • Placeholders remain throughout docs/investigations/discovery-http-server.md (<fill in>, <list>, <path:line>, <yes/no>, and <git rev-parse --short upstream/develop> as the checked-against commit).
  • TypeScript consumers are not identified. The table has one placeholder row and a TODO. If nothing outside tests consumes it, say so explicitly.
  • Python behaviour: the "Busy preferred port" row asks for probe output but none is pasted, and scripts/probe_discovery_port.py is not in the diff. Please also state which of OAUTH_DISCOVERY_PORT / discovery_port actually applies.
  • Comparison table has no filled values.
  • Conclusion still reads "Pick one and delete the other" with both options A and B present. Keep one; if B, link the follow-up implementation issue.
  • The PR description says consumers were identified and runtime behaviour verified, but the doc contains neither.
  • Minor: pin the actual commits checked, and add a trailing newline.

Please complete sections 1-4 with file:line references, then the single conclusion.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants