Skip to content

SEP-1789: Validate list_view column keys against the model the list route actually serializes - #1369

Open
peter-o-addo wants to merge 8 commits into
mainfrom
SEP-1789
Open

SEP-1789: Validate list_view column keys against the model the list route actually serializes#1369
peter-o-addo wants to merge 8 commits into
mainfrom
SEP-1789

Conversation

@peter-o-addo

@peter-o-addo peter-o-addo commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Reject TaskExecutionApp definitions where response_model disagrees with an explicit response_builder, so list-view column validation and the derived list route measure the same model.

  • app/sep/apps/framework/api.py, app/sep/apps/framework/apps.py: export resolve_response_model, add construction-time agreement validation between response_model and response_builder, and document that an explicit builder must declare the same return type.
  • docs/development/app-developer-guide.md: note that response_model must match an explicit response_builder's return annotation.
  • tests/app/sep/apps/framework/test_apps.py: cover mismatch rejection, matching annotation acceptance, unchanged TypeError for unannotated builders, and update the existing builder override fixture.

Tested

  • When response_builder returns a different model than response_model, app construction fails and the error mentions both model names.
  • When response_builder has no return type annotation, construction raises TypeError instead of ValueError.
  • When response_builder and response_model match, construction succeeds and the list route uses the builder’s model.
  • When no response_builder is provided, or when using a schema passthrough app, construction still succeeds unchanged.

Checklist

  • New/modified functions have type hints and rST docstrings
  • New tests added for new features or bug fixes
  • All tests pass locally (make test)
  • Pre-commit hooks pass (make run-pre-commit)
  • Database migrations generated if models changed (make makemigrations)
  • User-facing changes documented (README, inline help, UI text)
  • Configuration changes documented with examples
  • Changelog fragment added under changelog.d/ if the change is user-facing (make changelog-add), or confirmed N/A (internal-only change, or a same-release-cycle fix for an unreleased sibling ticket)

@peter-o-addo
peter-o-addo marked this pull request as ready for review August 19, 2026 14:56
Copilot AI balanced review requested due to automatic review settings August 19, 2026 14:56
@peter-o-addo
peter-o-addo requested review from a team and maxbube as code owners August 19, 2026 14:56
@peter-o-addo peter-o-addo added the qa in progress Someone is currently testing this PR - do not merge it label Aug 19, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Aligns TaskExecutionApp response-model validation with the model serialized by derived list routes.

Changes:

  • Exposes response-model resolution and validates builder/model agreement.
  • Documents the required response contract.
  • Adds mismatch, matching, and annotation tests.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
app/sep/apps/framework/api.py Exposes response-model resolution.
app/sep/apps/framework/apps.py Adds construction-time agreement validation.
docs/development/app-developer-guide.md Documents builder/model matching.
tests/app/sep/apps/framework/test_apps.py Covers validation behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread app/sep/apps/framework/apps.py Outdated
@peter-o-addo

Copy link
Copy Markdown
Contributor Author
  • 1) mismatch rejection names both models
[08/19/26 18:00:46] WARNING  app.core.auth.config: [-] The
                             top-level CASDOOR config is
                             deprecated; migrate to
                             AUTH__PROVIDER__CASDOOR__*
                             (AUTH.PROVIDER.CASDOOR). <80962>
Traceback (most recent call last):
  File "<stdin>", line 13, in <module>
  File "/Users/macbookpro/Documents/Percona/SEP/.venv/lib/python3.13/site-packages/pydantic/main.py", line 212, in __init__
    validated_self = self.__pydantic_validator__.validate_python(data, self_instance=self)
pydantic_core._pydantic_core.ValidationError: 1 validation error for TaskExecutionApp
  Value error, TaskExecutionApp: response_builder returns AltersTaskResponse but response_model declares BackupTaskResponse; the derived list route serializes the builder's model, so every response_model reader (the list_view column gate among them) measures the wrong object — set response_model to the builder's return type [type=value_error, input_value={'name': 'alters-mismatch...bject at 0x1118e38c0>,)}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.9/v/value_error
  • 2) unannotated builder still raises TypeError
build_alters_api_list_response(task: app.tasks.models.Task, *, status: app.tasks.models.TaskHistoryStatusEnum | None = None, last_executed_at: datetime.datetime | None = None, context: dict[str, str] | None = None) -> app.sep.apps.alters.models.AltersTaskResponse -> AltersTaskResponse
build_backup_pg_api_task_response(task: app.tasks.models.Task, *, status: app.tasks.models.TaskHistoryStatusEnum | None = None, last_executed_at: datetime.datetime | None = None, server_config: dict[str, typing.Any] | None = None) -> app.sep.apps.backup_pg.models.BackupTaskResponse -> BackupTaskResponse
  • 3) matching pair constructs; builder drives list model
[08/19/26 18:00:51] WARNING  app.core.auth.config: [-] The
                             top-level CASDOOR config is
                             deprecated; migrate to
                             AUTH__PROVIDER__CASDOOR__*
                             (AUTH.PROVIDER.CASDOOR). <81078>
alters: response_model=AltersTaskResponse
alters: builder_returns=AltersTaskResponse
alters: list_route.response_model=<class 'app.core.pagination.models.PaginatedResponse[AltersTaskResponse]'>

backup_pg: response_model=BackupTaskResponse
backup_pg: builder_returns=BackupTaskResponse
backup_pg: list_route.response_model=<class 'app.core.pagination.models.PaginatedResponse[BackupTaskResponse]'>
  • 4) unchanged default paths still construct
[08/19/26 18:03:09] WARNING  app.core.auth.config: [-] The
                             top-level CASDOOR config is
                             deprecated; migrate to
                             AUTH__PROVIDER__CASDOOR__*
                             (AUTH.PROVIDER.CASDOOR). <83807>
no_builder_api_router=True
schema_passthrough_api_router=True
  • 5) all registered task apps still construct
[08/19/26 18:03:09] WARNING  app.core.auth.config: [-] The
                             top-level CASDOOR config is
                             deprecated; migrate to
                             AUTH__PROVIDER__CASDOOR__*
                             (AUTH.PROVIDER.CASDOOR). <83810>
alters: api_router=True
backup_mongo: api_router=True
backup_mongo_restores: api_router=True
backup_pg: api_router=True
mysql_backups: api_router=True
mysql_backups_restores: api_router=True
archives: api_router=True
checksums: api_router=True
snippets: api_router=True

@peter-o-addo peter-o-addo added qa passed Tests for this PR are completed and successful. and removed qa in progress Someone is currently testing this PR - do not merge it labels Aug 19, 2026
peter-o-addo and others added 2 commits August 19, 2026 18:10
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python qa passed Tests for this PR are completed and successful.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants