[NI][Plugins] Derive Glean Server URL from work email in setup#9
[NI][Plugins] Derive Glean Server URL from work email in setup#9pragati-agrawal-glean wants to merge 1 commit into
Conversation
setup now accepts a work email and resolves the Server (QE) URL via the public config/search endpoint, instead of requiring the admin-only about-glean URL. Falls back to manual server_url entry when the domain isn't recognized; multi-tenant deployments resolve correctly. Bumps esbuild->0.28.1 and pins vite->7.3.5 (overrides) to clear CVE blocks that prevented the build from installing.
pragati-agrawal-glean
requested review from
eshwar-sundar-glean,
mohit-gupta-glean and
swarup-padhi-glean
| "stages: (1) resolve and save the Server URL, (2) authenticate, " + | ||
| "(3) fetch the remote tool catalog. Call with no arguments to advance " + | ||
| "through the next missing stage. Call with email to look up and " + | ||
| "configure your Glean instance automatically (the normal path). Call " + | ||
| "with server_url to set the Server URL directly (fallback when the " + | ||
| "email domain isn't recognized). Call with callback_url to finish " + | ||
| "authentication after a sign-in paste. Call with reset=true to clear " + |
There was a problem hiding this comment.
Can you test once with a dumber model like Sonnet with medium thinking to make sure it follows all these cases:
- Email + Auth + Reset
- Wrong email + wrong server url + correct server url + auth + reset
- Wrong email + correct server url + auth + reset
| `Invalid URL: "${rawUrl}". Please provide the Server instance (QE) URL ` + | ||
| `from https://app.glean.com/admin/about-glean ` + | ||
| `(e.g. https://acme-be.glean.com).`, | ||
| `(e.g. https://acme-be.glean.com), or pass your email instead.`, |
There was a problem hiding this comment.
#clarify
any reason to remove this line from https://app.glean.com/admin/about-glean
There was a problem hiding this comment.
It is removed because this link was admin-only, so the two available options are 1. to provide the qe-url directly (if they have) or give their email. The link is shown if they reach the fallback path (line #75). As per #9 (review), would show it only when email path does not work
|
|
||
| const CONFIG_SEARCH_URL = "https://app.glean.com/config/search"; | ||
|
|
||
| // Mirrors com/askscio config_endpoint DeploymentConfig. |
There was a problem hiding this comment.
#suggest
We may / may not accept contributions, its good to keep the repo self contained to public code and not reference implementation details / names from internal code base
| // Unknown domains aren't an error here — the endpoint returns the shared | ||
| // central URL with isMultiTenant=false. Reject that so a typo doesn't | ||
| // silently point at the central endpoint (the user can still use | ||
| // server_url). Multi-tenant customers legitimately have queryURL === | ||
| // centralURL but with isMultiTenant=true, so accept those. |
There was a problem hiding this comment.
clarify, what do you mean by this
Reject that so a typo doesn't
// silently point at the central endpoint (the user can still use
// server_url)
There was a problem hiding this comment.
So for customers whose isMultiTenant is true, it means they do not have a dedicated URL for them and are still on queryURL = centralURL = https://apps-be.glean.com/ . The comment here says that if someone does a typo in the email domain, that should not connect it to the centralURL but creates an error.
mohit-gupta-glean
left a comment
mohit-gupta-glean
left a comment
There was a problem hiding this comment.
#suggest Can we get rid of the fallback flow and simplify. We should document the server URL flag instead (in README, not in user journey). No point in complicating the setup and the fallback path anyway won't work for majority of users as they are not admins.
There was a problem hiding this comment.
rebase to latest master. The codex file is gone.
| `To connect this plugin to your Glean instance:\n` + | ||
| `To connect, either:\n` + | ||
| `- enter your work email (e.g. you@acme.com) and we'll find your Glean instance automatically, or\n` + | ||
| `- enter your Glean Server (QE) URL directly, if you have it.\n\n` + |
There was a problem hiding this comment.
#suggest can we not talk about the URL flow if email works. That means that only show this if email does not work for some reason.
|
|
||
| // Shown when an email's domain can't be resolved to a Glean instance. Falls | ||
| // back to the original manual method. | ||
| const MANUAL_URL_FALLBACK_TEXT = |
There was a problem hiding this comment.
#suggest Let's also add an option to try again with valid email to correct for typos.
| type: "string", | ||
| description: | ||
| "Your work email (e.g. you@acme.com). Used to look up your " + | ||
| "Glean Server instance (QE) URL automatically. Preferred over " + |
There was a problem hiding this comment.
This is very confusing. Here, we are saying preferred over server_url and then in server_url, we are saying "takes precedence over email".
3 participants
setup now accepts a work email and resolves the Server (QE) URL via the public config/search endpoint, instead of requiring the admin-only about-glean URL. Falls back to manual server_url entry when the domain isn't recognized; multi-tenant deployments resolve correctly.
Bumps esbuild->0.28.1 and pins vite->7.3.5 (overrides) to clear CVE blocks that prevented the build from installing.