chore: add agentsocket api client #20718
Conversation
23f500f to
01f7f04
Compare
af5fe5a to
f75b9fa
Compare
831a36b to
c300efb
Compare
f75b9fa to
53a0bb9
Compare
c300efb to
234bb48
Compare
eb34bc4 to
c8e8c20
Compare
remove unused types and simplify the interface
234bb48 to
72a8f70
Compare
mafredri
left a comment
There was a problem hiding this comment.
Since logic doesn't change much, this PR would normally be an easy approve, but fundamentally I think this might be the wrong approach. I think there should be a larger benefit than slight improvements in ergonomics of using the client as the burden to maintain it becomes greater. Are there any future plans I'm overlooking?
Ergonomics are a benefit. But I primarily include clients like these as an Anti Corruption Layer. It translates between abstractions layers. This feature is still in its early stages and I would like to isolate changes on one side of this client so they don't impact the other side as far as possible. I judge the benefits of such a layer to outweigh the cost of its maintenance, but if you feel strongly that its not the right move, we can remove it. It will be easy to take out once we have the CLI in place. How do you feel about deferring judgement on the removal of the client until we've seen the CLI use it? |
mafredri
left a comment
There was a problem hiding this comment.
This is now basically in approve territory, but I'm on the fence about the Windows change. (And one more suggestion.)
agent/agentsocket/client.go
Outdated
| } | ||
|
|
||
| // SyncStatus gets the status of a unit and its dependencies. | ||
| func (c *Client) SyncStatus(ctx context.Context, unitName string) (*SyncStatusResponse, error) { |
There was a problem hiding this comment.
For this case, I'd still advise to consider returning the proto types directly. This is a 1-to-1 conversion where the only change is where this type is defined.
If you disagree with the above, consider at least simplifying the return signature and dropping the pointer to response. Less indirection, safer to use, conveys that "this is not referenced elsewhere and safe to mutate".
…ndows despite the unsupported agentsocket feature
mafredri
left a comment
There was a problem hiding this comment.
Approved based on changes in next PR.
…20719) relates to: coder/internal#1094 This is number 5 of 5 pull requests in an effort to add agent script ordering. It adds CLI commands that allow coder scripts to manage dependencies between one another at runtime. I used an LLM to produce some of these changes, but I have conducted thorough self review and consider this contribution to be ready for an external reviewer. --------- Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
75771dc
into
jjs/internal-1095-branch3-agent-integration
relates to: coder/internal#1094
This is number 4 of 5 pull requests in an effort to add agent script ordering. It adds an API client that can call the workspace agent socket API for script ordering operations.
In a follow-up PR, CLI commands will be added that consume this API.
I used an LLM to produce some of these changes, but I have conducted thorough self review and consider this contribution to be ready for an external reviewer.