-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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