Skip to content

fix(qualify): refuse record.surface on non-surfaces verifier for every key shape - #2162

Open
yunusarda-sudo wants to merge 1 commit into
OpenAdaptAI:mainfrom
yunusarda-sudo:fix/record-surface-non-surfaces-refusal
Open

yunusarda-sudo wants to merge 1 commit into
OpenAdaptAI:mainfrom
yunusarda-sudo:fix/record-surface-non-surfaces-refusal

Conversation

@yunusarda-sudo

Copy link
Copy Markdown
Contributor

Resolves #2132.

Context & Problem

In #1576, record.surface support was introduced so a record contract can read other tables for its frame. However, record_reader validation (which verifies whether the verifier is kind: surfaces and whether the named surface exists) was only invoked via composite_key_read_problems, which only executed if isinstance(record.duplicate_key, list).

As noted in #2132, when record.duplicate_key was a scalar string (e.g. "id", "email") or unset (None), specifying record.surface on a non-surfaces verifier (e.g. kind: sql or kind: rest) was accepted silently by both qualify and verify_run parsers instead of raising a clear configuration error.

Solution

  • openadapt_flow/qualify/config.py (VerifyConfig._record): If record.surface is specified, validate it via record_reader(self.verifier.verifier, record.surface) and raise ValueError on problems before checking duplicate key structures.
  • openadapt_flow/verify_run.py (parse_verify_config): If record.surface is present, validate it via record_reader(cfg.verifier, surface) and raise VerifyRunError on problems for every key shape.
  • Tests (packages/flow/tests/test_record_surface.py):
    • Parametrized test across all key shapes (None, scalar "group_key", composite ["group_key"]) verifying that both qualify and verify_run parsers refuse record.surface on a non-surfaces verifier (record.surface needs verifier.kind: surfaces; verifier.kind is 'sql').
    • Parametrized test across all key shapes verifying that both parsers refuse an unknown surface name (record.surface names 'nowhere').

Verification

  • PYTHONPATH=packages/flow pytest packages/flow/tests/test_record_surface.py -v: 16/16 tests passed.
  • PYTHONPATH=packages/flow pytest packages/flow/tests/test_verify_run*.py packages/flow/tests/test_qualify*.py -v: 123/123 tests passed.
  • ruff check and ruff format --check: clean.

…y key shape (OpenAdaptAI#2132)

Signed-off-by: Yunus Ardaguney <yunusarda-sudo@users.noreply.github.com>

This branch has not been deployed

No deployments
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.

record.surface: refuse it on a non-surfaces verifier for every key shape

1 participant