- Notifications
You must be signed in to change notification settings - Fork927
feat: Add DevURL support#1316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
codecovbot commentedMay 6, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Codecov Report
@@ Coverage Diff @@## main #1316 +/- ##==========================================- Coverage 66.28% 58.06% -8.22%========================================== Files 281 284 +3 Lines 18438 18752 +314 Branches 220 235 +15 ==========================================- Hits 12221 10888 -1333- Misses 4962 6697 +1735+ Partials 1255 1167 -88
Continue to review full report at Codecov.
|
This adds apps as a property to a workspace agent.The resource is added to the Terraform provider here:coder/terraform-provider-coder#17Apps will be opened in the dashboard or via the CLIwith `coder open <name>`. If `command` is specified, aterminal will appear locally and in the web. If `target`is specified, the browser will open to an exposed instanceof that target.
| -- name: GetWorkspaceAppsByAgentID :many | ||
| SELECT * FROM workspace_apps WHERE agent_id = $1; | ||
| -- name: GetWorkspaceAppsByAgentIDs :many | ||
| SELECT * FROM workspace_apps WHERE agent_id = ANY(@ids :: uuid [ ]); | ||
| -- name: InsertWorkspaceApp :one | ||
| INSERT INTO | ||
| workspace_apps ( | ||
| id, | ||
| created_at, | ||
| agent_id, | ||
| name, | ||
| icon, | ||
| command, | ||
| target | ||
| ) | ||
| VALUES | ||
| ($1, $2, $3, $4, $5, $6, $7) RETURNING *; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If these are only specified in the template, why are they defined on a per-agent basis instead of a per-template basis?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
You could technically have a dynamic DevURL based on input parameters.
| @@ -645,6 +645,27 @@ func insertWorkspaceResource(ctx context.Context, db database.Store, jobID uuid. | |||
| if err != nil { | |||
| return xerrors.Errorf("insert agent: %w", err) | |||
| } | |||
| for _, app := range agent.Apps { | |||
| _, err := db.InsertWorkspaceApp(ctx, database.InsertWorkspaceAppParams{ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
What happens if an app is added to or removed from the template?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Because templates are versioned and so are our builds, we won't have these problems!
misskniss commentedMay 17, 2022
Is this PR still blocked by@deansheather's Port forwarding code? |
Yup, I'm reviewing his now! |
Abstracting coderd into an interface added misdirection becausethe interface was never intended to be fulfilled outside of a singleimplementation.This lifts the abstraction, and attaches all handlers to a root structnamed `*coderd.API`.
This adds DevURLs as a property to a workspace agent.
The resource is added to the Terraform provider here:
coder/terraform-provider-coder#17
DevURLs will be opened in the dashboard or via the CLI
with
coder open <name>. Ifcommandis specified, aterminal will appear locally and in the web. If
targetis specified, the browser will open to an exposed instance
of that target.
Todo:
coder opencommand to open a DevURL via the CLI.