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

Allow usage of plugin prefix on @eik/service #341

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mfolkeseth
Copy link

We need to to use Fastify plugin prefix https://www.fastify.io/docs/latest/Plugins/#route-prefixing-options and thus append pathname to host rather than replace.

In the current situation this request will erase the path and directly append pathname after hostname/prococol.

This proposal would append auth/login to the full server path given by the cli. If no plugin prefix is used, it will work with the default setup as it is today.

This proposal will allow:

eik login --server http://localhost:4001/with/some/prefix

We need to to use Fastify plugin prefix https://www.fastify.io/docs/latest/Plugins/#route-prefixing-options and thus append pathname to host rather than replace.

In the current situation this request will erase the path and directly append `pathname` after hostname/prococol.

This proposal would append `auth/login` to the full server path given by the cli. If no plugin prefix is used, it will work with the default setup as it is today.

This proposal will allow:

```
eik login --server http://localhost:4001/with/some/prefix
```
@digitalsadhu digitalsadhu requested a review from trygve-lie April 23, 2021 07:10
Copy link
Member

@digitalsadhu digitalsadhu left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! This looks fine to me. I can't any issue with it myself. I'll let @trygve-lie chime in before merging.

@trygve-lie
Copy link
Contributor

Thanks for this PR. Just so I'm sure I read you correct; you want the Server Rest API to be available at:

https://:assetServerUrl:port/:prefix/auth/login
https://:assetServerUrl:port/:prefix/pkg/:name/:version
etc..

Am I correct?

If yes, I do think we need to cater for this in the REST API too since we do a bit of logic on the URLs to find correct values.

I am very open to adding such a feature. I clearly see this is something others would need too.

@mfolkeseth
Copy link
Author

Yes, that is correct. When we use the fastify plugin prefix (referenced above), the case you are describing is what we get here. So we need the cli to act accordingly.

(I suspect you are somewhat familiar with our basePath structure at Amedia @trygve-lie)

@mfolkeseth
Copy link
Author

And by the way, I am more than happy to help out to make this happen as we are quite dependent on this feature. 👍

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.

3 participants