Commit Graph

13 Commits

Author SHA1 Message Date
Paweł Gronowski
2d69edd28a client/image_(inspect,history,load,save): Wrap return values
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-10-21 14:23:38 +02:00
Sebastiaan van Stijn
195a6bbb1e client: touch-up godoc
Not perfect yet, but addressing some godoc "doc" links that needed
to be updated, and touching up some references.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-08-02 17:12:05 +02:00
Sebastiaan van Stijn
4856e8ffad client: remove // import comments
These comments were added to enforce using the correct import path for
our packages ("github.com/docker/docker", not "github.com/moby/moby").
However, when working in go module mode (not GOPATH / vendor), they have
no effect, so their impact is limited.

Remove these imports in preparation of migrating our code to become an
actual go module.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-05-30 15:59:10 +02:00
Sebastiaan van Stijn
5af7c47f0e Merge pull request #49420 from thaJeztah/client_remove_serverResponse
client: remove serverResponse and use http.Response directly
2025-02-14 16:40:38 +01:00
Paweł Gronowski
903ba2f487 client: Move opts to separate files
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2025-02-14 14:45:08 +01:00
Paweł Gronowski
b9319f64ed client: Change ImageSave to use functional options
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2025-02-14 14:25:39 +01:00
Sebastiaan van Stijn
72c91e378d client: remove serverResponse and use http.Response directly
Looking in history to learn why this struct existed, shows that this type
was mostly the result of tech-debt accumulating over time;

- originally ([moby@1aa7f13]) most of the request handling was internal;
  the [`call()` function][1] would make a request, read the `response.Body`,
  and return it as a `[]byte` (or an error if one happened).
- some features needed the statuscode, so [moby@a4bcf7e] added an extra
  output variable to return the `response.StatusCode`.
- some new features required streaming, so [moby@fdd8d4b] changed the
  function to return the `response.Body` as a `io.ReadCloser`, instead
  of a `[]byte`.
- some features needed access to the content-type header, so a new
  `clientRequest` method was introduced in [moby@6b2eeaf] to read the
  `Content-Type` header from `response.Headers` and return it as a string.
- of course, `Content-Type` may not be the only header needed, so [moby@0cdc3b7]
  changed the signature to return `response.Headers` as a whole as a
  `http.Header`
- things became a bit unwieldy now, with the function having four (4) output
  variables, so [moby@126529c] chose to refactor this code, introducing a
  `serverResponse` struct to wrap them all, not realizing that all these
  values were effectively deconstructed from the `url.Response`, so now
  re-assembling them into our own "URL response", only preserving a subset
  of the information available.
- now that we had a custom struct, it was possible to add more information
  to it without changing the signature. When there was a need to know the
  URL of the request that initiated the response, [moby@27ef09a] introduced
  a `reqURL` field to hold the `request.URL` which notably also is available
  in `response.Request.URL`.

In short;

- The original implementation tried to (pre-maturely) abstract the underlying
  response to provide a simplified interface.
- While initially not needed, abstracting caused relevant information from
  the response (and request) to be unavailable to callers.
- As a result, we ended up in a situation where we are deconstructing the
  original `url.Response`, only to re-assemble it into our own, custom struct
  (`serverResponsee`) with only a subset of the information preserved.

This patch removes the `serverResponse` struct, instead returning the
`url.Response` as-is, so that all information is preserved, allowing callers
to use the information they need.

There is one follow-up change to consider; commit [moby@589df17] introduced
a `ensureReaderClosed` utility. Before that commit, the response body would
be closed in a more idiomatic way through a [`defer serverResp.body.Close()`][2].
A later change in [docker/engine-api@5dd6452] added an optimization to that
utility, draining the response to allow connections to be reused. While
skipping that utility (and not draining the response) would not be a critical
issue, it may be easy to overlook that utility, and to close the response
body in the "idiomatic" way, resulting in a possible performance regression.

We need to check if that optimization is still relevant or if later changes
in Go itself already take care of this; we should also look if context
cancellation is handled correctly for these. If it's still relevant, we could

- Wrap the the `url.Response` in a custom struct ("drainCloser") to provide
  a `Close()` function handling the draining and closing; this would re-
  introduce a custom type to be returned, so perhaps not what we want.
- Wrap the `url.Response.Body` in the response returned (so, calling)
  `response.Body.Close()` would call the wrapped closer.
- Change the signature of `Client.sendRequest()` (and related) to return
  a `close()` func to handle this; doing so would more strongly encourage
  callers to close the response body.

[1]: 1aa7f1392d/commands.go (L1008-L1027)
[2]: 589df17a1a/api/client/ps.go (L84-L89)
[moby@1aa7f13]: 1aa7f1392d
[moby@a4bcf7e]: a4bcf7e1ac
[moby@fdd8d4b]: fdd8d4b7d9
[moby@6b2eeaf]: 6b2eeaf896
[moby@0cdc3b7]: 0cdc3b7539
[moby@126529c]: 126529c6d0
[moby@27ef09a]: 27ef09a46f
[moby@589df17]: 589df17a1a
[docker/engine-api@5dd6452]: 5dd6452d4d

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2025-02-11 13:20:27 +01:00
Sebastiaan van Stijn
9c9eccfb23 client: support multiple platforms on save and load
We don't yet support this at the API level, so for now it returns
an error when trying to set multiple, but this makes sure that the
client types are already ready for this.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-11-19 15:56:28 +01:00
Sebastiaan van Stijn
96039276b6 client: add utilities to encode platforms
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-10-31 20:40:27 +01:00
Paweł Gronowski
f143f4ec51 image/save&load: Support Platform parameter
Add `Platform` parameter that allows to select a specific platform to
save/load.

This is a breaking change to the Go client as it changes the signatures
of `ImageLoad` and `ImageSave`.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-09-11 19:44:35 +02:00
Kir Kolyshkin
7d62e40f7e Switch from x/net/context -> context
Since Go 1.7, context is a standard package. Since Go 1.9, everything
that is provided by "x/net/context" is a couple of type aliases to
types in "context".

Many vendored packages still use x/net/context, so vendor entry remains
for now.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
2018-04-23 13:52:44 -07:00
Daniel Nephin
4f0d95fa6e Add canonical import comment
Signed-off-by: Daniel Nephin <dnephin@docker.com>
2018-02-05 16:51:57 -05:00
Michael Crosby
7c36a1af03 Move engine-api client package
This moves the engine-api client package to `/docker/docker/client`.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
2016-09-07 11:05:58 -07:00