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

Add configurable subpath for HTTP API #427

Open
stevekinney opened this issue Jul 1, 2024 · 1 comment
Open

Add configurable subpath for HTTP API #427

stevekinney opened this issue Jul 1, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@stevekinney
Copy link
Member

The change made in #410 causes a few potential problems for integrating with the UI.

First, some users host UI Server at a subpath and set both the client-side and API routes.

route.SetAPIRoutes(e, cfgProvider, serverOpts.APIMiddleware)
route.SetUIRoutes(e, cfg.PublicPath, assets)

Secondly, not having a sub-path for the API routes means that they will likely conflict with client-side UI routes. The current implementation of the UI passes through all routes with /api/v1 to UI Server. If both the pages and the API routes have the same path, it will be trickier to correctly implement the passthrough functionality.

Suggested resolutions:

  • The path for the API routes should be configurable.
  • They should default to something (e.g. /api) and not be set at the root by default since—selfishly—we'd need this set for CLI, Docker, and Temporal Cloud implementations anyway.
@stevekinney stevekinney added the enhancement New feature or request label Jul 1, 2024
@cretz
Copy link
Member

cretz commented Jul 1, 2024

The path for the API routes should be configurable.

I have opened temporalio/temporal#6212 (this is not done in this repo, it's part of the server that sets up the API)

The change made in #410 causes a few potential problems for integrating with the UI. [...] They should default to something (e.g. /api) and not be set at the root by default since

We do have stable roots of /namespaces, /cluster, and /system-info (opened #428 to handle one we missed). We should never add more than those three. We do not serve anything from the literal /, we just removed some unnecessary API path parts, not the whole thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants