Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

chore(appliance): Stub out react UI expected URIs and JSON API#63741

Merged
jdpleiness merged 23 commits into
mainfrom
jdp/appliance/react-ui-integration
Jul 15, 2024
Merged

chore(appliance): Stub out react UI expected URIs and JSON API#63741
jdpleiness merged 23 commits into
mainfrom
jdp/appliance/react-ui-integration

Conversation

@jdpleiness

@jdpleiness jdpleiness commented Jul 9, 2024

Copy link
Copy Markdown
Contributor

This PR stubs out the URI needed for the React UI to interface with the appliance, as well as removed the previously implemented UI and components of the React UI that were only around for a demo.

A number of helper and safety methods have also been added for interfacing with JSON reads/writes and handling common errors.

While the HTTP handlers are still only stubs, this PR was growing in size so I decided to cut it here and break apart the rest in upcoming PRs. React UI is able to parse status and auth correctly at this time.

Test plan

Unit tests

Changelog

@cla-bot cla-bot Bot added the cla-signed label Jul 9, 2024
@jdpleiness jdpleiness changed the title feat(appliance): Stub out react UI expected URIs and JSON API chore(appliance): Stub out react UI expected URIs and JSON API Jul 9, 2024
@jdpleiness jdpleiness added the no-changelog Exclude this PR from the next changelog. label Jul 10, 2024
@jdpleiness jdpleiness requested a review from craigfurman July 10, 2024 12:59
Comment thread internal/appliance/json.go Outdated
Comment on lines 43 to 77

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Mostly Cody here and some previous JSON API work mixed in so maybe double check. Either way it's just error handling.

Comment on lines 22 to 41

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Moved this and a few other things out of a separate pkg just to consolidate. Plan to delete this whole package as well once the appliance API is properly wired.

@jdpleiness jdpleiness force-pushed the jdp/appliance/react-ui-integration branch from 8b9b4e5 to 8b194ac Compare July 10, 2024 14:16

@craigfurman craigfurman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good start! I haven't actually kicked the tires on the integration since this is still a draft, but it's good to see it taking shape 🙌

Comment thread internal/appliance/json.go Outdated
Comment thread internal/appliance/json.go Outdated
Comment thread internal/appliance/errors.go Outdated
Comment thread internal/appliance/errors.go Outdated
Comment thread go.mod Outdated

@craigfurman craigfurman Jul 10, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we might want to keep the password strength validation, but can always push that to follow-on issue since there's been a lot of upheaval in this area lately 😅

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'll re-add this after this PR

Comment thread internal/appliance/routes.go Outdated
Comment thread internal/appliance/json.go Outdated
Comment thread internal/appliance/json.go Outdated
@jdpleiness jdpleiness force-pushed the jdp/appliance/react-ui-integration branch from 68a53a1 to e66165f Compare July 12, 2024 20:31
@jdpleiness jdpleiness force-pushed the jdp/appliance/react-ui-integration branch from eeeb8eb to bf38dcb Compare July 15, 2024 01:38
@jdpleiness jdpleiness marked this pull request as ready for review July 15, 2024 02:34
@jdpleiness jdpleiness requested review from a team and Chickensoupwithrice and removed request for a team July 15, 2024 02:34

@craigfurman craigfurman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great work! 🚀

I left a few notes but didn't want to block on any of them, especially since we'll be iterating fast in this area probably. I'll leave it up to you which ones to address and which to punt.

"sigs.k8s.io/yaml"

"github.com/sourcegraph/log"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

optional / maybe for follow-on: we can remove the JWT secret generation from this file

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ahh I missed that! 🙇

Comment thread internal/appliance/auth_test.go Outdated
Comment thread internal/appliance/errors.go Outdated
}), nil
}

func (a *Appliance) postSetupHandler() http.Handler {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

From the PR description, am I right in thinking that the plan is for a follow-on to reintroduce installation?

Comment thread internal/appliance/json.go Outdated
Comment thread internal/appliance/json.go
@jdpleiness jdpleiness merged commit b71c986 into main Jul 15, 2024
@jdpleiness jdpleiness deleted the jdp/appliance/react-ui-integration branch July 15, 2024 18:48
craigfurman pushed a commit that referenced this pull request Jul 31, 2024
This PR stubs out the URI needed for the React UI to interface with the
appliance, as well as removed the previously implemented UI and
components of the React UI that were only around for a demo.

A number of helper and safety methods have also been added for
interfacing with JSON reads/writes and handling common errors.

While the HTTP handlers are still only stubs, this PR was growing in
size so I decided to cut it here and break apart the rest in upcoming
PRs. React UI is able to parse status and auth correctly at this time.

<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->

## Test plan

Unit tests

## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed no-changelog Exclude this PR from the next changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants