fix(trainer): PodmanClientAdapter.list_containers crashes when filters is None
- Dominant language
- Python
- Stars
- 148
- Forks
- 262
- Avg merge
- 1d 2h
- Merged PRs (30d)
- 1
Description
### What happened?
Was testing `PodmanClientAdapter.list_containers()` and it breaks when called without filters, even though filters is supposed to be optional.
Here's the interface:
```python
def list_containers(
self, filters: dict[str, list[str]] | None = None
) -> list[dict]:
```
But the implementation never checks for `None`, it just goes straight into:
```python
for k, v in filters.items():
if len(v) == 1:
filters[k] = v[0]
try:
containers = self.client.containers.list(all=True, filters=filters)
```
So when filters isn't passed, it defaults to `None`, and `None.items()` throws:
### What did you expect to happen?
`list_containers()` should work fine without filters, that's the whole point of it being optional.
```python
adapter.list_containers()
```
Should return all containers, same as passing an empty dict. Shouldn't throw an AttributeError before it even gets to `self.client.containers.list()`.
### Environment
Kubernetes version:
```bash
$ kubectl version
```
N/A — this bug is in `PodmanClientAdapter.list_containers()`, which fails before any Kubernetes or Podman API call is made. No cluster is involved in reproducing it.
Kubeflow Trainer version:
```bash
$ kubectl get pods -n kubeflow -l app.kubernetes.io/name=trainer -o jsonpath="{.items[*].spec.containers[*].image}"
```
N/A — not running Trainer in a cluster for this. Using the `kubeflow/sdk` repository directly at:
- Branch: `main`
- Commit: `3febcd7377d0cdb322bbb420207c3c26cbf7afd7`
- Affected file: `kubeflow/trainer/backends/container/adapters/podman.py`
Kubeflow Python SDK version:
```bash
$ pip show kubeflow
```
N/A — not using an installed release. Running from the repository source at the commit above (`kubeflow/sdk`, branch `main`).
Additional notes:
- Python 3.13.7
- Podman: `pyproject.toml` declares `podman>=5.6.0` as an optional dependency
- Reproduced using a mocked Podman client, no Podman daemon required
### Impacted by this bug?
Impacted by this bug?
Contributor guide
Assessment
This issue has not been assessed yet.