Skip to content

client: refactor service api client functions for defined options/res…#51252

Merged
austinvazquez merged 1 commit intomoby:masterfrom
austinvazquez:refactor-client-service
Oct 22, 2025
Merged

client: refactor service api client functions for defined options/res…#51252
austinvazquez merged 1 commit intomoby:masterfrom
austinvazquez:refactor-client-service

Conversation

@austinvazquez
Copy link
Contributor

…ult structs

- What I did

Part of #50965

This change refactors the client ServiceAPIClient interface implementations to use client module defined option structs and results to simplify future development of the client.

Non-functional touchups from #51244

- How I did it

- How to verify it

- Human readable description for the release notes

- A picture of a cute animal (not mandatory but encouraged)

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if CI is happy 😄

Comment on lines +72 to +80
func newTaskLogsResult(rc io.ReadCloser) TaskLogsResult {
if rc == nil {
panic("nil io.ReadCloser")
}
return TaskLogsResult{
rc: rc,
close: sync.OnceValue(rc.Close),
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK for a follow-up; I highly suspect we may have existing utilities for this; possibly we could either reuse those or make something "generic".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol I wrote a generic logResults type but then thought perhaps I was overcooking. 🤣

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah; we should look what's best we can do that later!

@thaJeztah
Copy link
Member

@austinvazquez looks like something broke somewhere

@austinvazquez
Copy link
Contributor Author

Yeah going through the delta now to see what I missed.

@austinvazquez austinvazquez force-pushed the refactor-client-service branch from a4aa4c1 to 0d46e66 Compare October 21, 2025 22:36
@austinvazquez
Copy link
Contributor Author

I believe I found the issue in a test cleanup. An inspect was being deferred instead of a remove service call causing conflicts with other tests.

@thaJeztah
Copy link
Member

🙈 needs a rebase (sorry!)

@thaJeztah thaJeztah added the release-blocker PRs we want to block a release on label Oct 21, 2025
@thaJeztah thaJeztah added this to the 29.0.0 milestone Oct 21, 2025
…ult structs

Co-authored-by: Claude <noreply@anthropic.com>
Signed-off-by: Austin Vazquez <austin.vazquez@docker.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/dependencies area/logging area/networking Networking kind/refactor PR's that refactor, or clean-up code module/client release-blocker PRs we want to block a release on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants