APP-6785: Remove local control page (RC) #4520

ethanlookpotts · 2024-11-04T18:58:07Z

We are removing RC from the RDK - users are now expected to use app.viam.com to control their machine.

Note

We don't expect workflows to pass until the base PR has been merged.

Change log

Remove web/frontend
Remove unused code in web/
Remove unused e2e script
Remove web-related targets from Makefile
Remove web-related docs from README

Review requests

Please review robot/web/web.go thoroughly - I aimed to remove anything related to serving the web app, but I might've trimmed too much or missed some code. Let me know!
Anyone missing from the reviewer list?

ethanlookpotts · 2024-11-04T19:03:28Z

Makefile

+build: build-go

 build-go:


I could collapse build-go into build

Suggested change

build: build-go

build-go:

build:

I think it's fine this way. I just double checked the proposed makefile with the current github action workflows (to the degree grepping for "make" and "build"/"test" gets what I need).

Github actions are directly using build-go and test-go. So if you go ahead with this change:

You'll need to update the github action files invoking make build-go.

make build will be "go-less". But make {lint,test,generate} will have "go-ful" versions that github actions use.

ethanlookpotts · 2024-11-04T19:03:47Z

Makefile

@@ -55,7 +47,7 @@ tool-install:
 		github.com/rhysd/actionlint/cmd/actionlint \
 		golang.org/x/tools/cmd/stringer

-lint: lint-go lint-web
+lint: lint-go


I could collapse lint-go into lint

ethanlookpotts · 2024-11-05T18:13:21Z

robot/web/web.go

-	if err := svc.installWeb(mux, svc.r, options); err != nil {
-		return nil, err
-	}
+	mux.Handle(pat.New("/"), http.NotFoundHandler())


@Otterverse see here - not found for /. How do health checks work for the agent?

It looks for a 2XX status code on whatever URL viam-server gives as it's "serving" line during startup. Ex:

11/5/2024, 12:35:22 PM info rdk web/web.go:599 serving url https://artoo-main.p55vcc4g4x.local.viam.cloud:8080 alt_url https://0.0.0.0:8080 11/5/2024, 12:35:22 PM info viam-agent viamserver/viamserver.go:172 healthcheck URLs: https://artoo-main.p55vcc4g4x.local.viam.cloud:8080 https://localhost:8080 11/5/2024, 12:35:22 PM info viam-agent viamserver/viamserver.go:173 viam-server started

Basically agent sees that first line and parses it, and reports WHAT it parsed in that second line. Then it queries that every minute, looking for a 2XX http response (it doesn't care about the response body.)

Awesome! I added a 200 status and "healthy" message here: 596b486

@Otterverse are you able to verify that this works if I generate an AppImage?

Agent doesn't use appimages. Those are the "old" method. Just do a static binary make server-static and then you can test it yourself. You can copy the built file to your device (running agent) and set the pin_url for viam-server to file:///path/to/local/viam-server to let it run the new version. (You can also use a remote/public URL if you find that easier.)

I can try the same now too... will clone your branch and give it a whirl.

Okay, running a build from this branch, looks good!

2024-11-05T20:19:31.030Z INFO rdk web/web.go:449 serving {"url":"https://artoo-main.p55vcc4g4x.local.viam.cloud:8080","alt_url":"https://0.0.0.0:8080"} 2024-11-05T20:19:31.031Z INFO viam-agent viamserver/viamserver.go:172 healthcheck URLs: https://artoo-main.p55vcc4g4x.local.viam.cloud:8080 https://localhost:8080 ... 2024-11-05T20:20:34.390Z DEBUG viam-agent viamserver/viamserver.go:290 healthcheck for viam-server is good 2024-11-05T20:20:34.391Z DEBUG viam-agent agent/manager.go:233 Subsystem healthcheck succeeded for viam-server

Sorry for the delay here - thank you for taking a look! I am also going to try going through these steps for my understanding. Once I've confirmed that things are working as expected I'll move forward with the base PR and put this one up for explicit review.

Sweet! I was able to verify this as well and everything is behaving as expected.

dgottlieb · 2024-11-08T14:09:08Z

We are removing RC from the RDK - users are now expected to use app.viam.com to control their machine.

So this breaks using localhost:8080 ? Is there some alternative workflow for robots using a "local FS" config?

benjirewis · 2024-11-08T14:26:59Z

Was also looking @dgottlieb ; assume this is relevant https://viam.atlassian.net/wiki/spaces/ENG/pages/829587457/Local+control+page

ethanlookpotts · 2024-11-08T14:29:40Z

Correct @dgottlieb, this breaks using localhost:8080. The confluence page that @benjirewis linked is how you can connect with cloud signaling, but not sure if it works in the "local FS" case. Trying now.

dgottlieb · 2024-11-08T14:36:16Z

Thanks for that document link! But I'm not sure if that helps me? If I wanted to avoid using app, I don't think running a local app instance is the direction I was hoping for.

ethanlookpotts · 2024-11-08T14:38:31Z

If I wanted to avoid using app, I don't think running a local app instance is the direction I was hoping for.

It doesn't run the whole local app instance - just the control tab, just like the page on localhost:8080 used to be. The code does live in the app repo, but you don't run the whole app. Acknowledged that it doesn't help OSS users, only our employees at the moment.

web/app_fs.go

randhid · 2024-11-08T15:38:04Z

robot/web/web.go

-type AppTemplateData struct {
-	WebRTCEnabled          bool                   `json:"webrtc_enabled"`
-	WebRTCSignalingAddress string                 `json:"webrtc_signaling_address"`
-	Env                    string                 `json:"env"`
-	Host                   string                 `json:"host"`
-	StaticHost             string                 `json:"static_host"`
-	SupportedAuthTypes     []string               `json:"supported_auth_types"`
-	AuthEntity             string                 `json:"auth_entity"`
-	BakedAuth              map[string]interface{} `json:"baked_auth"`
-}


can new local app handle no credentials passed, insecure, like the one you're deleting did?

I can not run rdk without credentials and use the sdks to connect to it with an insecure option, is this possible in the new world where we build "local" rc?

Insecure connections haven't been built into the svelte-sdk. I can add them if it would be useful to you

This is what @dgottlieb is looking for as well - we need a small TS SDK change in order to provide this from the svelte-sdk (aka the new local control page). Opened here, and will also open the corresponding change against the svelte-sdk.

ethanlookpotts · 2024-11-08T19:32:00Z

This is still ready for review but I will wait to merge it until we have the solution for the new local control page connecting insecurely to a local machine running a local config. (This PR will not make the next viam-server release.)

dgottlieb

I saw the TS changes were merged. Would it be helpful for me to test run a local config with the new proposed flow?

ethanlookpotts · 2024-11-12T14:57:50Z

I saw the TS changes were merged. Would it be helpful for me to test run a local config with the new proposed flow?

@dgottlieb Not quite yet, but soon! We need the TS SDK to be bumped in App (happening this afternoon) and then we can move forward with https://github.com/viamrobotics/app/pull/6824. Then I will update the instructions and ask you to test the new workflow for me! Thank you!

randhid

Let me know if you need someone to manually test the local app build with insecure credentials when that is ready.

ethanlookpotts · 2024-11-21T22:05:37Z

I've verified with @dgottlieb and @randhid that the new local control page in App works for their needs.

I'm going to go ahead and merge this tomorrow so it makes it into the next RDK release. Please review if needed!

benjirewis · 2024-11-21T22:09:09Z

Taking myself off review here. Makes sense/LGTM though.

viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Nov 4, 2024

ethanlookpotts mentioned this pull request Nov 4, 2024

APP-6785: Remove local control page - remove web workflows #4523

Merged

ethanlookpotts force-pushed the APP-6785/ethanlookpotts/remove-rc branch from 1d271d1 to 60f837f Compare November 4, 2024 20:17

viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Nov 4, 2024

ethanlookpotts changed the base branch from main to APP-6785/ethanlookpotts/remove-web-workflows November 4, 2024 20:17

ethanlookpotts commented Nov 4, 2024

View reviewed changes

ethanlookpotts force-pushed the APP-6785/ethanlookpotts/remove-rc branch from 60f837f to 8aa32a5 Compare November 4, 2024 21:21

randhid mentioned this pull request Nov 5, 2024

[RSDK-7904] Remove status from components #4461

Open

1 task

ethanlookpotts commented Nov 5, 2024

View reviewed changes

ethanlookpotts added the appimage-ignore-tests Build AppImage of PR and ignore tests label Nov 5, 2024

Base automatically changed from APP-6785/ethanlookpotts/remove-web-workflows to main November 7, 2024 22:31

APP-6785: Remove local control page (RC)

fefba9d

ethanlookpotts force-pushed the APP-6785/ethanlookpotts/remove-rc branch from 596b486 to fefba9d Compare November 7, 2024 22:43

viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels Nov 7, 2024

ethanlookpotts marked this pull request as ready for review November 7, 2024 22:44

ethanlookpotts requested review from dgottlieb, benjirewis, randhid and zaporter-work November 7, 2024 22:45

randhid reviewed Nov 8, 2024

View reviewed changes

dgottlieb approved these changes Nov 12, 2024

View reviewed changes

randhid approved these changes Nov 18, 2024

View reviewed changes

benjirewis removed their request for review November 21, 2024 22:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

APP-6785: Remove local control page (RC) #4520

APP-6785: Remove local control page (RC) #4520

ethanlookpotts commented Nov 4, 2024 •

edited

Loading

ethanlookpotts Nov 4, 2024

dgottlieb Nov 12, 2024

ethanlookpotts Nov 4, 2024

ethanlookpotts Nov 5, 2024

Otterverse Nov 5, 2024

ethanlookpotts Nov 5, 2024

Otterverse Nov 5, 2024

Otterverse Nov 5, 2024

Otterverse Nov 5, 2024

ethanlookpotts Nov 7, 2024

ethanlookpotts Nov 7, 2024

dgottlieb commented Nov 8, 2024

benjirewis commented Nov 8, 2024

ethanlookpotts commented Nov 8, 2024

dgottlieb commented Nov 8, 2024

ethanlookpotts commented Nov 8, 2024

randhid Nov 8, 2024 •

edited

Loading

zaporter-work Nov 8, 2024 •

edited

Loading

ethanlookpotts Nov 8, 2024

ethanlookpotts commented Nov 8, 2024 •

edited

Loading

dgottlieb left a comment

ethanlookpotts commented Nov 12, 2024 •

edited

Loading

randhid left a comment

ethanlookpotts commented Nov 21, 2024

benjirewis commented Nov 21, 2024

APP-6785: Remove local control page (RC) #4520

Are you sure you want to change the base?

APP-6785: Remove local control page (RC) #4520

Conversation

ethanlookpotts commented Nov 4, 2024 • edited Loading

Change log

Review requests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgottlieb commented Nov 8, 2024

benjirewis commented Nov 8, 2024

ethanlookpotts commented Nov 8, 2024

dgottlieb commented Nov 8, 2024

ethanlookpotts commented Nov 8, 2024

randhid Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

zaporter-work Nov 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethanlookpotts commented Nov 8, 2024 • edited Loading

dgottlieb left a comment

Choose a reason for hiding this comment

ethanlookpotts commented Nov 12, 2024 • edited Loading

randhid left a comment

Choose a reason for hiding this comment

ethanlookpotts commented Nov 21, 2024

benjirewis commented Nov 21, 2024

ethanlookpotts commented Nov 4, 2024 •

edited

Loading

randhid Nov 8, 2024 •

edited

Loading

zaporter-work Nov 8, 2024 •

edited

Loading

ethanlookpotts commented Nov 8, 2024 •

edited

Loading

ethanlookpotts commented Nov 12, 2024 •

edited

Loading