You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I think I see what's happening here, for the remove issue. (TL;DR from that issue: it looks like the remove flag is not honored in some cases)
After a short investigation, I see that there are two different flags auto_remove and remove. The intended semantics, I think, are as follows:
remove is supposed to instruct the podman-py client library to spawn a thread to wait for the container to run and then remove it (after perhaps extracting the logs -- this wouldn't be possible with a daemon-side remove)
auto_remove is instead the flag that enables the remove API flag, which in turn maps to the HostConfig.AutoRemove attribute of a container that causes its removal after its termination.
For the create operation, remove is filtered out and NOT handled at all. So it DOES NOT result in the removal of the container after its execution, albeit being documented to do so. As far as I can see, the implementation makes no attempt at all to use this flag.
For the run operation, instead, the flag is partially handled and does result in the client-side removal of the container, after extracting its logs if requested by the caller -- but only in the synchronous (attach=True) case. In the asynchronous case, there is no way to extract the logs.
For both operations, auto_remove is correctly mapped to the API flag, so no issues there.
To address these limitations, I think the following would be the best course of action:
remove the remove flag from the create operation's docstring. The reason it's not implemented, is that it doesn't really make a whole lot of sense there. A use case for run, is to run a container, wait for it (synchronously or asynchronously), and then extract its output. The remove flag makes sense for this usage as a convenience flag to avoid having to handle container cleanup manually in the caller. Imho the only reason why create currently accepts remove, is to avoid having to filter it out before having run internally call create. It should never have been documented
update the documentation for the run operation, detailing the semantics and limitations of the remove flag
I will be submitting a PR corresponding to the above proposals.
The text was updated successfully, but these errors were encountered:
I'm splitting off for #493.
I think I see what's happening here, for the
remove
issue. (TL;DR from that issue: it looks like theremove
flag is not honored in some cases)After a short investigation, I see that there are two different flags
auto_remove
andremove
. The intended semantics, I think, are as follows:remove
is supposed to instruct thepodman-py
client library to spawn a thread to wait for the container to run and then remove it (after perhaps extracting the logs -- this wouldn't be possible with a daemon-side remove)auto_remove
is instead the flag that enables theremove
API flag, which in turn maps to theHostConfig.AutoRemove
attribute of a container that causes its removal after its termination.For the
create
operation,remove
is filtered out and NOT handled at all. So it DOES NOT result in the removal of the container after its execution, albeit being documented to do so. As far as I can see, the implementation makes no attempt at all to use this flag.For the
run
operation, instead, the flag is partially handled and does result in the client-side removal of the container, after extracting its logs if requested by the caller -- but only in the synchronous (attach=True) case. In the asynchronous case, there is no way to extract the logs.For both operations,
auto_remove
is correctly mapped to the API flag, so no issues there.To address these limitations, I think the following would be the best course of action:
remove
flag from thecreate
operation's docstring. The reason it's not implemented, is that it doesn't really make a whole lot of sense there. A use case forrun
, is to run a container, wait for it (synchronously or asynchronously), and then extract its output. Theremove
flag makes sense for this usage as a convenience flag to avoid having to handle container cleanup manually in the caller. Imho the only reason whycreate
currently acceptsremove
, is to avoid having to filter it out before havingrun
internally callcreate
. It should never have been documentedrun
operation, detailing the semantics and limitations of theremove
flagI will be submitting a PR corresponding to the above proposals.
The text was updated successfully, but these errors were encountered: