-
-
Notifications
You must be signed in to change notification settings - Fork 658
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
Docs: update API access for new token type #1958
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
Coverage report
Test suite run success1146 tests passing in 188 suites. Report generated by 🧪jest coverage report action from 8927e8d |
cf7fdfc
to
5d69657
Compare
53723ad
to
eb760ff
Compare
ac55474
to
3012950
Compare
@Tymek Sorry! The review isn't done yet 🙇🏼 The overall comment was also just a draft and not meant to be posted just yet. I'll continue now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, sorry about the double review, but I finally got through everything 😅 Great work on this! 🙌🏼 I've got a few nits here, but it's mostly small fry and I've tried to include suggestions for everything, so it should hopefully be easy to see what I mean or to just add my suggestions if you agree.
If there's anything you disagree with or the like, please don't hesitate to let me know.
Regarding the comment that I accidentally added to my previous review, please let me expand on it a bit:
The 'username' field of the API token creation form is confusing, because we're not actually asking for a username at all. We're asking the users what they'd like to call the token. (We've had questions about this!) I'm personally very surprised that the field apparently isn't required to be unique either. So for that, I think this is the time to change the name of the field in the application to something like "token identifier" or "token name". What do you think?
Left to do
Before it can go in, I think we also have to update the api tokens and client keys article with:
- A detailed description of what settings are used to create an API token (username, projects, environments, token type)
- A new section on FRONTEND tokens
Discussion
Are we happy with using "frontend" as the url for the API (and the token type etc)? Is that the name of it? It seems like we're calling the API the "direct access API" (unless I misunderstood, that is), so maybe that's more appropriate? 🤷🏼
frontend/src/component/admin/apiToken/CreateApiToken/CreateApiToken.tsx
Outdated
Show resolved
Hide resolved
`website/docs/user_guide/token.mdx` Co-authored-by: Thomas Heartman <thomas@getunleash.ai>
`website/docs/topics/frontend-api.md` Co-authored-by: Thomas Heartman <thomas@getunleash.ai>
Co-authored-by: Thomas Heartman <thomas@getunleash.ai>
Co-authored-by: Thomas Heartman <thomas@getunleash.ai>
|
||
### Cross-origin resource sharing (CORS) configuration {#cors} | ||
|
||
You need to allow traffic from your application domains to use the Unleash front-end API with web and hybrid mobile applications. You can update the front-end API CORS settings from the Unleash UI under _admin \> CORS_ or by using the API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Tymek I almost forgot about this! What's the API for this? We probably won't have the docs until the OpenAPI integration is ready, but what's the url? /api/admin/cors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How to set it with admin API? I think it's api/admin/ui-config
, POST with frontend settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's what I was wondering about. Seems strange that it's a UI config thing, though?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super nice work! 🙌🏼 let's get this in 😄 I think we can handle the one thing about the API later once we get the openapi docs in.
About the changes
Documentation for roadmap item #1875 (Embedded proxy)
https://linear.app/unleash/issue/1-152/embedded-proxy-documentation