Skip to content

fix(pebble): avoid pydantic warning for exec checks - #1196

Open
gcomneno wants to merge 4 commits into
canonical:mainfrom
gcomneno:contrib/issue-1154-pydantic-warning
Open

fix(pebble): avoid pydantic warning for exec checks#1196
gcomneno wants to merge 4 commits into
canonical:mainfrom
gcomneno:contrib/issue-1154-pydantic-warning

Conversation

@gcomneno

@gcomneno gcomneno commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • make the check discriminator handle already-validated check models during serialization
  • avoid the PydanticSerializationUnexpectedValue warning when marshalling projects with exec checks
  • add a unit test covering Project.marshal() with an exec health check

Issue

@gcomneno
gcomneno requested a review from tigarmo as a code owner April 23, 2026 02:00
@gcomneno
gcomneno force-pushed the contrib/issue-1154-pydantic-warning branch from 37fbe6a to ec259e3 Compare May 24, 2026 16:18
@gcomneno

Copy link
Copy Markdown
Contributor Author

Hi! Just gently following up on this one.

I rebased the branch onto current upstream/main and re-ran the focused checks locally:

  • uv run --group dev python -m pytest -q tests/unit/test_project.py -rs
  • uv run --group lint --group types ruff check rockcraft/pebble.py tests/unit/test_project.py
  • uv run --group lint --group types ruff format --check rockcraft/pebble.py tests/unit/test_project.py

The focused test run passes with 80 passed, and Ruff is clean.

Happy to adjust anything else if useful.

lczyk and others added 2 commits June 3, 2026 09:58
narrow check to Mapping before key lookup and build the tag list by
membership instead of set intersection so the return type stays str;
cast the marshalled checks dict in the new test.

@lczyk lczyk left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gcomneno thanks for the PR! :) looks good. i've let the CI run and noticed the linting errors, so here is a PR to your PR where i've tried to placate our type-checking overlords. ff to use or ignore. 🤷‍♀️

not gonna 'request changes' since the PR is sound, but dont wanna quite 'approve' since the linting wants some care.

@tigarmo bump. i'm poking at this because just run into the same thing

@gcomneno

gcomneno commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @lczyk, I merged your patch and reran the focused checks locally.

Validation:

  • uv run --group dev python -m pytest -q tests/unit/test_project.py -rs → 80 passed
  • uv run --group lint --group types ruff check rockcraft/pebble.py tests/unit/test_project.py
  • uv run --group lint --group types ruff format --check rockcraft/pebble.py tests/unit/test_project.py
  • git diff --check

The branch is now updated at 1d898ec.

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.

Pydantic warning: PydanticSerializationUnexpectedValue

2 participants