Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Container inspection fails with podman #472

Closed
ambasta opened this issue Sep 4, 2023 · 3 comments · Fixed by #473
Closed

Container inspection fails with podman #472

ambasta opened this issue Sep 4, 2023 · 3 comments · Fixed by #473
Labels
podman Specific to podman

Comments

@ambasta
Copy link
Contributor

ambasta commented Sep 4, 2023

In components.container.models.ContainerConfig, stop_signal only accepts strings. Podman otoh, responds with StopSignal: 15, i.e. integer values.

The line

python-on-whales/python_on_whales/components/container/models.py

Line 218 in d885139

stop_signal: Optional[str] = None

should probably be stop_signal: Optional[str | int] = None
@gabrieldemarmiesse
Copy link
Owner

Thanks for the report! I'm open to a pull request with the fix and a unit test (we have some dummy json files that we try to load into the model, we just need to add one example there). If no one does the PR, I'll make it myself but I don't guarantee it's going to be quick.

ambasta added a commit to ambasta/python-on-whales that referenced this issue Sep 4, 2023
@ambasta
fix(ContainerConfig): stop-signal can be str or int
51062f2 
With podman, `container inspect` returns signal codes as integers, for
example "StopSignal": 15

The current typing is `Optional[str]` which when enforced via pydantic,
fails to parse response from podman.

Fixes gabrieldemarmiesse#472

Signed-off-by: Amit Prakash Ambasta <amit.prakash.ambasta@gmail.com>
@ambasta ambasta mentioned this issue Sep 4, 2023
Merged
@ambasta
Copy link
Contributor Author

ambasta commented Sep 4, 2023

Sure, fired a PR. Is there a way I can test the PR locally before pushing again (without having docker installed)

@gabrieldemarmiesse
Copy link
Owner

Sadly we only support docker for unit testing but I would like some day to support podman and nerdctl too.

I'll take a look at the PR, thanks!

@LewisGaul LewisGaul added the podman Specific to podman label Dec 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
podman Specific to podman
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(ContainerConfig): stop-signal can be str or int ambasta/python-on-whales
3 participants
@ambasta @gabrieldemarmiesse @LewisGaul