Skip to content

fix(core): Refactor copy file#996

Merged
Tranquility2 merged 3 commits intomainfrom
update_copy_file
Apr 1, 2026
Merged

fix(core): Refactor copy file#996
Tranquility2 merged 3 commits intomainfrom
update_copy_file

Conversation

@Tranquility2
Copy link
Copy Markdown
Contributor

@Tranquility2 Tranquility2 commented Apr 1, 2026

Follow-up to #852, cleaning up the copy file feature after it landed.
(Related to #676)

A few things stood out:

  • _transfer_file_content_into_container and _transfer_directory_into_container did basically the same thing (build a tar, upload it) but were separate methods with duplicated logic
  • assert was used for error handling in production code, these silently vanish under python -O, so real failures would just pass through unnoticed
  • copy_from_container treated each HTTP chunk from get_archive() as its own tar file, works fine for small files that fit in one chunk, breaks silently for anything larger
  • The tar-building logic was stuffed inside DockerContainer when it really has nothing to do with container lifecycle, it's just data transformation on a Transferable

@Tranquility2 Tranquility2 changed the title Refactor copy file core: Refactor copy file Apr 1, 2026
@Tranquility2 Tranquility2 changed the title core: Refactor copy file fix(core): Refactor copy file Apr 1, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 1, 2026

Codecov Report

❌ Patch coverage is 80.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.24%. Comparing base (898faf6) to head (6a621a4).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
core/testcontainers/core/container.py 46.15% 3 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #996      +/-   ##
==========================================
- Coverage   79.54%   79.24%   -0.30%     
==========================================
  Files          15       15              
  Lines        1281     1277       -4     
  Branches      151      154       +3     
==========================================
- Hits         1019     1012       -7     
- Misses        218      219       +1     
- Partials       44       46       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[[package]]
name = "testcontainers"
version = "4.14.1"
version = "4.14.2"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

??

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we should update this with release-please

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oh this is unsolved problem. what happens if we ignore it? i guess uv will fail to install.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it was somehow missed 🤷

@Tranquility2 Tranquility2 merged commit 0e0bb24 into main Apr 1, 2026
9 checks passed
@Tranquility2 Tranquility2 deleted the update_copy_file branch April 1, 2026 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants