Codecov Report

❌ Patch coverage is 93.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.54%. Comparing base (2ca2321) to head (d032a55).
⚠️ Report is 1 commits behind head on main.

@@            Coverage Diff             @@
##             main     #852      +/-   ##
==========================================
+ Coverage   78.86%   79.54%   +0.67%     
==========================================
  Files          14       15       +1     
  Lines        1221     1281      +60     
  Branches      142      151       +9     
==========================================
+ Hits          963     1019      +56     
- Misses        216      218       +2     
- Partials       42       44       +2     

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

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Hi @rhoban13, what is the current status of this feature? What could I do to help?

For what it's worth, I think the suggested interface is clear. I haven't tested it with my use cases yet though.

Tagging a few maintainers seeking feedback @terry-docker @alexanderankin @Tranquility2

i think my current priorities are docs, anything that makes this library behave like the others (this PR is illustrating current state not improving it), so far the only new issue from the new release is someone complaining about warning logs - so i think that means that the new release was successful (either that or there are no more users of this library). anecdotally someone reached out on slack asking for a new release but they are only allowed to use it at work a week after it has been released. so there may be new issues in some time.

wait, this adds Transferable - this is great - if we can also fix the immediate starting of the container this almost entirely fixes copying. just have to maybe see about creating folders automatically and i think we may have parity on copying files with the other impls

Yes I was just about to update the PR title - my original intent was just some tests, but I found it easier to write those tests using TDD & just implement it at the same time. I think this brings the library a bit closer to feature parity with some others.

I'll take a look at adding some tests around folders if it sounds like you're in agreement with the interface at a high level.

Thanks! I'll try to run some tests then and will let you know how they went.

if we can also fix the immediate starting of the container

This is independent of this PR, right? I presume you're referring to the fact that we're calling docker run instead of create+start?

yeah this whole API is not going to behave as intended until we start doing create then start

I ran some tests with this PR, it went well 🙂.

I used this function to transfer directories:

def transferables_from_directory(
    source_dir: Path, dest_dir: Path, mode: int
) -> list[TransferSpec]:
    transferables: list[TransferSpec] = []
    for dirpath, _, filenames in source_dir.walk():
        for name in filenames:
            origin = dirpath / name
            destination = dest_dir / dirpath.relative_to(source_dir) / name
            transferables.append((origin, str(destination), mode))

    return transferables

I updated the implementation so all these methods should work if the transferable Path is a directory as well.

going to ping @Tranquility2 in an attempt to make sure this PR doesnt get forgotten - I want either this or the DB stuff to be the next release (sometime in the next couple weeks)

going to ping @Tranquility2 in an attempt to make sure this PR doesnt get forgotten - I want either this or the DB stuff to be the next release (sometime in the next couple weeks)

How can I assist?