Skip to content

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Jan 13, 2025


depends on these fixes to be released in docker-py;


pkg/jsonmessage: stop printing deprecated progressDetail, errorDetail

The API still returns it for backward-compatibility (but probably
shouldn't), but we should no longer print it. This removes the
use of these fields for printing, but keeps them for streamformatter
to use.

  • ErrorMessage was deprecated in 3043c26
  • ProgressMessage was deprecated in 597e0e6

client/pkg/jsonmessage: remove DisplayJSONMessagesToStream

It was an adaptor around DisplayJSONMessagesStream for CLI-specific
primitives that was used in the CLI, but can be implemented by users
of this package.

client/pkg/jsonmessage: remove Stream interface

It was an interface to match CLI-specific primitives and is no
longer used.

- What I did

- How I did it

- How to verify it

- Description for the changelog

`client/pkg/jsonmessage`: remove deprecated `ProgressMessage`, `ErrorMessage`, `DisplayJSONMessagesToStream` and `Stream` interface

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

@thaJeztah thaJeztah added area/api API status/2-code-review kind/refactor PR's that refactor, or clean-up code area/go-sdk impact/go-sdk Noteworthy (compatibility changes) in the Go SDK labels Jan 13, 2025
@thaJeztah thaJeztah added this to the 28.0.0 milestone Jan 13, 2025
@thaJeztah thaJeztah self-assigned this Jan 13, 2025
@thaJeztah thaJeztah force-pushed the jsonmessage_remove_deprecated branch from d4db89a to 73b0797 Compare January 13, 2025 13:20
@thaJeztah
Copy link
Member Author

docker-py error could be legit, which means it's depending on the deprecated fields;

=================================== FAILURES ===================================
_____________________ ImageCollectionTest.test_load_error ______________________
tests/integration/models_images_test.py:96: in test_load_error
    with pytest.raises(docker.errors.ImageLoadError):
E   Failed: DID NOT RAISE <class 'docker.errors.ImageLoadError'>
------- generated xml file: /src/bundles/test-docker-py/junit-report.xml -------

@thaJeztah thaJeztah modified the milestones: 28.0.0, 29.0.0 Feb 11, 2025
@thaJeztah thaJeztah force-pushed the jsonmessage_remove_deprecated branch from 73b0797 to 58048d0 Compare July 14, 2025 16:23
@thaJeztah thaJeztah force-pushed the jsonmessage_remove_deprecated branch 2 times, most recently from 69eb214 to 8d8ecd8 Compare July 23, 2025 12:32
@thaJeztah thaJeztah force-pushed the jsonmessage_remove_deprecated branch 2 times, most recently from f8967b1 to dac3a63 Compare July 30, 2025 11:24
@thaJeztah thaJeztah changed the title pkg/jsonmessage: JSONMessage: remove deprecated ProgressMessage, ErrorMessage pkg/jsonmessage: stop printing deprecated progressDetail, errorDetail Jul 30, 2025
@thaJeztah thaJeztah force-pushed the jsonmessage_remove_deprecated branch 2 times, most recently from 1a4ae99 to 45d370c Compare July 30, 2025 12:35
The API still returns it for backward-compatibility (but probably
shouldn't), but we should no longer print it. This removes the
use of these fields for printing, but keeps them for streamformatter
to use.

- ErrorMessage was deprecated in 3043c26
- ProgressMessage was deprecated in 597e0e6

Signed-off-by: Sebastiaan van Stijn <[email protected]>
It was an adaptor around DisplayJSONMessagesStream for CLI-specific
primitives that was used in the CLI, but can be implemented by users
of this package.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
It was an interface to match CLI-specific primitives and is no
longer used.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah force-pushed the jsonmessage_remove_deprecated branch from 45d370c to 2b4506b Compare July 30, 2025 21:23
@thaJeztah thaJeztah changed the title pkg/jsonmessage: stop printing deprecated progressDetail, errorDetail pkg/jsonmessage: stop printing deprecated progressDetail, errorDetail, remove DisplayJSONMessagesToStream and Stream interface Jul 30, 2025
@thaJeztah thaJeztah force-pushed the jsonmessage_remove_deprecated branch from 2b4506b to f3ba0b2 Compare July 30, 2025 21:24
@thaJeztah
Copy link
Member Author

I have some other changes in a local branch, but need to give those some thinking to see if we can still provide aliases with those. Effectively, I want to un-export most bits, and swap out the DisplayJSONMessagesStream for an iterator / walker that can be used to render JSONMessages. It would be similar to how the auxCallback functions are used; if none is provided, we can provide the default (which is the current way things are rendered), but users can pass their own callback(s) (possibly multiple to allow writing to multiple outputs, although that could be handled internally in their render function / callback.

@thaJeztah thaJeztah marked this pull request as ready for review July 30, 2025 21:39
@thaJeztah thaJeztah merged commit 0a5fb23 into moby:master Jul 31, 2025
188 of 189 checks passed
@thaJeztah thaJeztah deleted the jsonmessage_remove_deprecated branch July 31, 2025 00:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api API area/go-sdk impact/go-sdk Noteworthy (compatibility changes) in the Go SDK kind/refactor PR's that refactor, or clean-up code status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants