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

backend: change the default backend plugin mount point to /api #2562

Merged
merged 1 commit into from
Sep 24, 2020

Conversation

Rugvip
Copy link
Member

@Rugvip Rugvip commented Sep 22, 2020

Switches things to be mounted on /api in the backend. This is a bit extra painful because we don't have #1847 in place, so there are a couple of hardcoded routes in some places in the backends.

Expecting that as long as all dependencies are bumped in sync, the only breakage is that OAuth redirect URLs need to be reconfigured. It is somewhat possible to keep the existing routing in place, and I added a parameter for that in the auth backend, but there is some new hardcoded usage of /api.

I did investigate creating more of an API around this in the service build from backend-common, but tbh it just got awkward with ordering of routes, and there's also a pretty minimal of code required anyway. I did also try making the service build do a bit more work in creating new environments, but figured that can be explored more easily after this would have been merged.

@Rugvip
Copy link
Member Author

Rugvip commented Sep 24, 2020

Should clarify that the primary reason for doing this is to enable deployments where the frontend and backend are hosted on the same origin, as currently e.g. /catalog is both used by the frontend and backend.

Copy link
Member

@OrkoHunter OrkoHunter left a comment

Choose a reason for hiding this comment

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

cc @spotify/techdocs-core

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.

None yet

3 participants