feat/Appliance: Frontend Pulls Version Info from RelReg#64089
Conversation
|
This feels like the wrong approach tbh, calling relregistry from clientside, when we have a release registry client in the backend is a bit silly. But then I'd have to expose the api endpoint on the backend that calls another api endpoint (the relreg) Doing it this way means we need to pass another env var to the front end, which isn't the end of the world, but is annoying. i'm also still struggling to get a build of the maintaince ui running to test this, so i'm not actually sure it works, hence it's a draft |
|
Just remembered that I need to do the whole CORS dance 😭 |
craigfurman
left a comment
There was a problem hiding this comment.
Yeah, this errors in my browser due to lack of CORS bypass.
Sadly we appear to have lost unified instructions on how to kick the tires on the appliance in development since we introduced the new UI.
You should be able to test this by: run the appliance backend, using instructions in https://github.com/sourcegraph/sourcegraph/blob/main/cmd/appliance/README.md.
In another terminal:
cd internal/appliance/frontend/maintenance
pnpm i
pnpm run dev
Assuming you get no build errors, navigate to localhost:8889 in your browser and you should see the maintenance UI.
|
Thanks for the tips @craigfurman, I managed to get a test environment setup :) However it looks like CORS headers aren't being set on the releaseregistry, so I'm gonna make a PR there too |
|
Alright, finally got this tested, and working :) 
Requires changes to releaseregistry such that it also passes CORS headers, which I've made here |
craigfurman
left a comment
There was a problem hiding this comment.
Nice work! 🚀
I haven't reproduced the testing since (if I understand correctly) I'd need to populate some data in a local relreg until the CORS headers PR lands.
| const [version, setVersion] = useState<string[]>([]) | ||
| const [selectedVersion, setSelectedVersion] = useState<string>('') | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
I have zero react experience so take this with a tablespoon of salt, but this view file doesn't feel like quite the right place for this function. Maybe we put it in a ts file and import it? Totally non-blocking note in any case.
There was a problem hiding this comment.
This is pretty typical in react with hooks in my experience
| const data = await response.json() | ||
| setVersion(data) | ||
| if (data.length > 0) { | ||
| const publicVersions = data.filter(item => item.public).map(item => item.version) |
There was a problem hiding this comment.
The critical bit here 👍🏼
<!-- PR description tips: https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e --> Resolves [REL-290](https://linear.app/sourcegraph/issue/REL-290/frontend-ui-pulls-sourcegraph-versions-to-install-from-release) by making a single request to the release registry to get a list of versions ## Test plan manual tested <!-- REQUIRED; info at https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles --> ## Changelog <!-- OPTIONAL; info at https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c --> - feat(appliance): frontend pulls versions from relreg
Resolves REL-290 by making a single request to the release registry to get a list of versions
Test plan
manual tested
Changelog