-
Notifications
You must be signed in to change notification settings - Fork 905
feat(agent/agentcontainers): add Exec method to devcontainers CLI #18244
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
dda97d9
to
2984d4e
Compare
d49f84e
to
011a8aa
Compare
ae52380
to
5cf4ae7
Compare
011a8aa
to
63f93bc
Compare
5cf4ae7
to
2dc395d
Compare
63f93bc
to
0deaab8
Compare
2dc395d
to
ea0125d
Compare
0deaab8
to
8796ba3
Compare
// WithContainerID sets the container ID to target a specific container. | ||
// Can only be used with the Exec command. | ||
func WithContainerID(id string) DevcontainerCLIOptions { | ||
return func(o *devcontainerCLIUpConfig) { | ||
if o.command != "exec" { | ||
panic("developer error: WithContainerID can only be used with the Exec command") | ||
} | ||
o.args = append(o.args, "--container-id", id) | ||
} | ||
} | ||
|
||
// WithRemoteEnv sets environment variables for the Exec command. | ||
// Can only be used with the Exec command. | ||
func WithRemoteEnv(env ...string) DevcontainerCLIOptions { | ||
return func(o *devcontainerCLIUpConfig) { | ||
if o.command != "exec" { | ||
panic("developer error: WithRemoteEnv can only be used with the Exec command") | ||
} | ||
for _, e := range env { | ||
o.args = append(o.args, "--remote-env", e) | ||
} | ||
} | ||
} | ||
|
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.
Given that these two options can only be used with devcontainer exec
, I think it makes more sense to keep them as a different type. I like the idea of having the uniform signature of ...DevcontainerCLIOptions
but this is offset by potential run-time errors instead of compile-time errors.
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.
Yeah, agreed, I'll change it.
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.
Fixed in adbfd45
This change adds Exec to the devcontainer CLI. Updates coder/internal#621
8796ba3
to
adbfd45
Compare
This change adds Exec to the devcontainer CLI.
Updates coder/internal#621