Skip to content

fix: preserve list-valued filters - #657

Open
Shubham-Padkonde wants to merge 1 commit into
containers:mainfrom
Shubham-Padkonde:fix/list-valued-filters
Open

Shubham-Padkonde wants to merge 1 commit into
containers:mainfrom
Shubham-Padkonde:fix/list-valued-filters

Conversation

@Shubham-Padkonde

@Shubham-Padkonde Shubham-Padkonde commented Sep 22, 2026 •

Copy link
Copy Markdown

Fixes #542: dictionaries containing list-valued filters now encode each element as a separate filter value rather than serializing the entire Python list as one string. Scalar conversion remains unchanged; empty lists and None values contribute no criteria.

Tests cover multiple labels, mixed scalar/list filters, numeric elements, empty/None entries, and the actual ContainersManager.list HTTP query without mutating the caller's filter dictionary.

Validation:

  • Seven formatter subcases and the HTTP regression fail before and pass after locally.
  • Hosted Fedora 44 unittest-coverage check passes: 257 tests run, one skipped, 85% reported total coverage. Test log.
  • Local Windows/Python 3.13 broader run: 257 passed, 2 skipped, 13 failed; original production code has the same 13 failures. test_utils.py collection needs /etc/os-release and a Podman executable and was excluded from that local comparison.
  • Ruff lint/format, targeted mypy and git diff --check pass locally.

Remaining hosted checks are not all green: distro-sanity errors before lint with /bin/sh: - : invalid option; Fedora 43 and rawhide RPM builds failed (build logs could not yet be retrieved), and several distro integration checks are pending. The passing Linux unit/coverage check resolves the original local validation blocker; these remaining checks still need investigation before merge.

Prepared with OpenAI Codex assistance. No local Podman daemon or container data was modified.

@Shubham-Padkonde
Shubham-Padkonde marked this pull request as ready for review September 22, 2026 05:42
@Shubham-Padkonde

Copy link
Copy Markdown
Author

Fixed the test-case type inference reported by the sanity mypy job in 177df45. The explicit case annotation preserves all test inputs and runtime behavior.

Validation: tox -e py -- podman/tests/unit/test_api_utils.py -q passed 13 tests and 7 subtests; Ruff formatting and lint passed; mypy --platform linux --package podman passed for all 85 source files. The type check was run on Windows with the Linux target because native Windows checking reports the existing POSIX-only os.getuid/geteuid attributes.

Prepared with OpenAI Codex assistance; the contributor personally signed off this commit.

@Honny1 Honny1 left a comment

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.

Please sqoush comiints in to one. I have just one qestion about type.

PTAL @timcoding1988

Comment thread podman/api/http_utils.py Outdated
criteria[key].append(str_value)
else:
criteria[key] = [str_value]
values = value if isinstance(value, list) else [value]

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.

What if the value is a tuple?

@Shubham-Padkonde

Copy link
Copy Markdown
Author

Updated the filter handling to preserve tuples as well as lists and extended the regression cases to cover both. Squashed the three personally signed commits into 1685e6f as requested; verified the squashed file tree is identical to the signed pre-squash tree.

The prepared patch passed 13 focused tests with 14 subtests, Ruff and the Linux-target mypy check before signing. Full hosted distribution/integration checks are running and are not claimed passed.

Prepared with Codex assistance.

@Shubham-Padkonde

Copy link
Copy Markdown
Author

I checked all five failing distribution jobs on 1685e6f. They share five URL-history assertions comparing percent-encoded socket paths (%2F versus %2f); those same five tests also fail in the saved baseline run without this filter change. Fedora 44, 45 and rawhide additionally fail test_ssh_ping with a connection reset by peer. Fedora 43 and CentOS Stream 10 show only the five URL assertions.

The new filter cases pass, as do unit coverage, sanity, DCO and all seven RPM builds. I have kept the patch scoped to filter handling; the distribution checks are still red. Example full log: https://artifacts.dev.testing-farm.io/91afd380-fa62-4e9f-b33e-bcf8cb585534/work-all_python_6llwv4x/plans/distro/all_python/execute/data/guest/default-0/tests/tests/all_python-1/output.txt

Prepared with Codex assistance.

@Honny1 Honny1 left a comment

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.

Please add Fixes #542 to the commit message. Thanks!

PS: I am working on ci fix.

Fixes containers#542

Signed-off-by: Shubham Padkonde <shubhampadkonde12@gmail.com>

This branch was successfully deployed

1 active (outdated) deployment
build — 177df457 Deployed Sep 23, 2026 by Shubham-Padkonde via Test Build Python distribution 📦 #231
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.

ContainersManager.list() with label list filter doesn't perform filtering correctly

2 participants