docs: looker oauth implementation example #995
Conversation
+1 to this!!! |
|
for sure, it's on my todo list for this PR. |
There was a problem hiding this comment.
Testing issue.
- Left form field empty and hit enter. Login enabled.
- Clicked login and it hung on the configuration page. Totally stuck at this point. I suspect I have to delete something from local storage
Suggest:
- change UI to form field and single button. Button protected if form field is not a valid URL.
- Ping versions endpoint to validate URL?
More testing
Deleted local storage and entered valid URL. Routed me to server and I logged in successfully. Back to /oauth and hangs
| "scripts": { | ||
| "develop": "webpack serve --host=0.0.0.0 --https --disable-host-check --config webpack.dev.config.js" | ||
| }, | ||
| "dependencies": { |
There was a problem hiding this comment.
My concern about the examples is the difficulty we have keeping the versions up to date. We have an issue with the access token server which I'm thinking should be moved to packages and eventually deleted when artifacts is done.
I don't know of a better place to put the example. I've been thinking of writing a script for examples that allows me to update versions easily. May be the script should go here and it can be run after each publish and eventually automated? Perhaps a script to ensure that examples have been updated to the current version when we do a publish. Maybe we can have a CI job update the packages post publish.
Thinking out loud here. Totally unrelated to the merits of this PR.
There was a problem hiding this comment.
I think you've created this script now, @bryans99?
|
|
||
| export const ConfigKey = 'serverConfig' | ||
|
|
||
| export const initSdk = () => { |
There was a problem hiding this comment.
This could go into extension-utils (which should be renamed). It can accept configKey, baseUrl, agentTag and maybe track performance as arguments
There was a problem hiding this comment.
agreed, see #1000
There was a problem hiding this comment.
I also agree that we should rename extension-utils. We should discuss with @jkaster
There was a problem hiding this comment.
Sorry, I don't know how I missed this before. Maybe I was on vacation? Anyway, what are suggested other names?
| <Route path="/oauth"> | ||
| <OAuthScene adaptor={adaptor} /> | ||
| </Route> | ||
| <Route path="/" exact> |
There was a problem hiding this comment.
What happens if path not '/' and not '/oauth'? Can we remove the path and exact?
There was a problem hiding this comment.
this is something I did not think of. I will eliminate the "/" path so that it can handle anything.
|
This PR is still very much a draft. I will polish it a bit more tomorrow, please hold off on re-reviewing. TODOs:
|
No description provided.