Skip to content

put_archive: note the data may also be a stream#2478

Merged
milas merged 1 commit intodocker:mainfrom
valohai:put-archive-docs
Feb 22, 2023
Merged

put_archive: note the data may also be a stream#2478
milas merged 1 commit intodocker:mainfrom
valohai:put-archive-docs

Conversation

@akx
Copy link
Copy Markdown
Contributor

@akx akx commented Dec 10, 2019

The docs imply the data parameter to put_archive() must be bytes (a bytestring), which is not the case; a stream is fine too since it's passed directly to Requests.

In fact, the tests for the put_archive() feature always pass in a stream:

docker-py/tests/integration/api_container_test.py

Lines 678 to 679 in a0b9c3d

with helpers.simple_tar(test_file.name) as test_tar:
self.client.put_archive(ctnr, '/vol1', test_tar)

@felixfontein
Copy link
Copy Markdown
Contributor

felixfontein commented Jan 25, 2020

This is also something I noticed recently. I've taken a look at the Requests adapters provided by docker-py, it doesn't look like they do anything funky which will prevent this. But I'm not really familiar with these aspects of Requests (and the adapters) to be able to say this with full confidence :)

@akx
Copy link
Copy Markdown
Contributor Author

akx commented Jan 27, 2020

@felixfontein Well, it works for me in production as we speak 😁

@felixfontein
Copy link
Copy Markdown
Contributor

felixfontein commented Jan 28, 2020

@akx out of curiosity, which transports are you using it with / did you test it with? There's unix, tcp, tcp+tls, ssh+unix, and npipe (Windows only).

@akx
Copy link
Copy Markdown
Contributor Author

akx commented Jan 28, 2020 •
edited
Loading

@felixfontein Plain UNIX in this instance.

@akx akx force-pushed the put-archive-docs branch from 6d1afa8 to 78f3693 Compare July 14, 2022 13:38
@damwiw
Copy link
Copy Markdown

damwiw commented Jan 12, 2023

Hi,
After a code review of request.py and Go docker client, I can confirm:

  • request accepts streams (documentation: file-like objects) as data field. iter and read methods are expected.
  • docker cp command with stdin as input uses such a mechanism.
    By the way, tar archive may be a tar gzipped archive.
    Sucessfully tested with 4 Go of compressed tar archive.

@akx
put_archive: note the data may also be a stream
542f3fd 
Signed-off-by: Aarni Koskela <akx@iki.fi>
@akx akx force-pushed the put-archive-docs branch from 78f3693 to 542f3fd Compare February 3, 2023 17:29
@akx
Copy link
Copy Markdown
Contributor Author

akx commented Feb 3, 2023

Rebased. Could a maintainer take a look?

milas
milas approved these changes Feb 22, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@milas milas left a comment

Choose a reason for hiding this comment

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

Thanks for rebasing, let's get this merged 🙂

@milas milas merged commit aaf68b7 into docker:main Feb 22, 2023
@akx akx deleted the put-archive-docs branch December 21, 2023 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aiordache aiordache Awaiting requested review from aiordache

@ulyssessouza ulyssessouza Awaiting requested review from ulyssessouza

1 more reviewer

@milas milas milas approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@akx @felixfontein @damwiw @milas