Skip to content

Conversation

@thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Mar 9, 2025

@thaJeztah thaJeztah added status/2-code-review kind/refactor PR's that refactor, or clean-up code labels Mar 9, 2025
@codecov-commenter
Copy link

codecov-commenter commented Mar 9, 2025

Codecov Report

Attention: Patch coverage is 77.50000% with 9 lines in your changes missing coverage. Please review.

Project coverage is 59.16%. Comparing base (410c0ba) to head (7cc6b8e).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5914      +/-   ##
==========================================
+ Coverage   59.12%   59.16%   +0.04%     
==========================================
  Files         355      355              
  Lines       29740    29733       -7     
==========================================
+ Hits        17583    17591       +8     
+ Misses      11182    11168      -14     
+ Partials      975      974       -1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@thaJeztah thaJeztah force-pushed the use_atomicwriter branch 4 times, most recently from 5e6b61d to 64a2e41 Compare March 25, 2025 10:50
@thaJeztah thaJeztah force-pushed the use_atomicwriter branch 4 times, most recently from 5e54fa9 to 7d1f7bf Compare April 4, 2025 21:11
@thaJeztah thaJeztah added this to the 28.0.5 milestone Apr 4, 2025
Same functionality, but implemented with atomicwriter. There's a slight
difference in error-messages produced (but can be adjusted if we want).

Before:

    docker image save -o ./no/such/foo busybox:latest
    failed to save image: invalid output path: directory "no/such" does not exist

    docker image save -o /no/permissions busybox:latest
    failed to save image: stat /no/permissions: permission denied

After:

    docker image save -o ./no/such/foo busybox:latest
    failed to save image: invalid file path: stat no/such: no such file or directory

    docker image save -o /no/permissions busybox:latest
    failed to save image: failed to stat output path: lstat /no/permissions: permission denied

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Same functionality, but implemented with atomicwriter. There's a slight
difference in error-messages produced (but can be adjusted if we want).

Before:

    docker container export -o ./no/such/foo mycontainer
    failed to export container: invalid output path: directory "no/such" does not exist

    docker container export -o /no/permissions mycontainer
    failed to export container: stat /no/permissions: permission denied

After:

    docker container export -o ./no/such/foo mycontainer
    failed to export container: invalid file path: stat no/such: no such file or directory

    docker container export -o /no/permissions mycontainer
    failed to export container: failed to stat output path: lstat /no/permissions: permission denied

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah modified the milestones: 28.0.5, 28.1.0 Apr 10, 2025
@thaJeztah thaJeztah marked this pull request as ready for review April 10, 2025 07:47
@thaJeztah
Copy link
Member Author

@vvoland @Benehiko ptal 🤗

Comment on lines +28 to 29
// Deprecated: use [atomicwriter.New].
func CopyToFile(outfile string, r io.Reader) error {
Copy link
Member Author

Choose a reason for hiding this comment

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

This was used in compose, but has been removed there in docker/compose#12715

@thaJeztah thaJeztah merged commit 8633197 into docker:master Apr 11, 2025
92 checks passed
@thaJeztah thaJeztah deleted the use_atomicwriter branch April 11, 2025 10:10
@thaJeztah thaJeztah self-assigned this May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

3 participants