fix(grz-common,grz-db,grzctl): move populate logic into SubmissionDb#580
Merged
Conversation
Hoeze
changed the title
Hoeze
force-pushed
the
fix/move-populate-to-grz-db
branch
from
af0f578 to
3c66ecf
Compare
Pure rename to keep git's similarity heuristic happy. The follow-up commit refactors the contents to exercise SubmissionDb.populate directly instead of Worker.populate.
Hoeze
force-pushed
the
fix/move-populate-to-grz-db
branch
5 times, most recently
from
55ce5bf to
859267f
Compare
`grz-common/workers/worker.py` previously imported from `grz_db.models.submission` at module top, but `grz-common`'s pyproject did not declare `grz-db` as a dependency. The transitive-only import broke isolated installs of `grz-cli` with `ModuleNotFoundError: No module named 'grz_db'`, surfacing as a conda-forge build failure. Lift the DB-touching code out of grz-common entirely: - Add `SubmissionDb.populate` (and the private static `_log_pending_changes` helper) to `grz_db.models.submission`. Takes the parsed metadata, the S3 last-modified date, `force`, and an optional logger. - Drop `Worker.populate` and `Worker._log_pending_changes` from grz-common. Replace them with `Worker.load_metadata_with_submission_date`, which keeps the S3 `head_object` + metadata.json read inside grz-common (no grz-db touch). - Wire `grzctl/commands/download.py` to the new two-step API: `worker.load_metadata_with_submission_date(...)` followed by `db.populate(...)`. - Refactor the previously-renamed `test_db_populate.py` to exercise `SubmissionDb.populate` directly, dropping the S3 client mock and the Worker construction. The grzctl click `populate` command at `commands/db/cli.py:654` is unaffected. grz-common no longer has any `grz_db` references in source code, so no pyproject changes are required.
`SubmissionDb.populate` previously had no way to skip fields during diff or to bypass the redacted-`tan_g` / missing-`local_case_id` guards. The grzctl click `populate` command exposes an `--ignore-field` flag and threads it into `db.diff(ignore_fields=...)` plus its own redaction-guard gating, but the orchestrator method could not match that capability. Add an `ignore_fields: set[str] | None = None` keyword-only parameter: - Forwarded to `self.diff(...)` so the same field-suppression semantics apply (the diff layer already supports it). - Gates the redacted-TAN check on `"tan_g" not in ignore_fields` and the missing/redacted-`local_case_id` check on `"pseudonym" not in ignore_fields`, matching the strings the click command already uses. The download path (`grzctl/commands/download.py`) leaves the parameter unset and keeps the previous behavior. Tests in `tests/cli/test_db_populate.py` cover both the raise-when-not-ignored and skip-when-ignored cases for both fields, plus a spy assertion that `ignore_fields` reaches `db.diff`.
`SubmissionDb.populate` and the grzctl click `db submission populate` command both performed identical redaction guards (redacted `tan_g` and missing/redacted `local_case_id`, each gated by the same `ignore_fields` string keys). Extract the shared check into a `SubmissionDb.assert_metadata_not_redacted` static method. - `SubmissionDb.populate` now delegates to `assert_metadata_not_redacted`. - The click command catches the raised `ValueError` and re-raises a `click.ClickException` with the CLI-specific hint (mention of the metadata path and the `--ignore-field` flag) so the user-facing error text stays useful while the check itself lives in one place. - `test_populate_redacted` updated to assert against the click error path rather than a raw `ValueError` traceback.
Move the "head_object on $submission_id/metadata/metadata.json -> timestamp" operation into a free function in `grz_common.transfer`, alongside `init_s3_client`. `Worker.load_metadata_with_submission_date` calls it and narrows the result to a `date` for storage in the DB. Documents the inbox-vs-archive caveat: the S3 `LastModified` of the metadata.json object is the authoritative inbox-receipt timestamp, but once the submission has moved to the archive bucket the `LastModified` no longer reflects the time of submission. Callers operating against the archive (e.g. `grzctl db backfill`) must therefore not use this helper and must keep deferring to the in-file `submission.submissionDate` field. The existing `_backfill_submission` already adds "submission_date" to `ignore_fields`, so this is already the case in practice.
Hoeze
force-pushed
the
fix/move-populate-to-grz-db
branch
from
859267f to
2c137b6
Compare
Four SubmissionNotFoundError branches told the user to run 'grz-cli db submission add', but that subcommand lives in grzctl, not grz-cli. Following the hint gives 'no such command'. Replace the binary name in all four messages.
…in populate The 'already up to date' branch in 'db submission populate' raised SystemExit via ctx.exit() and then had an unreachable 'return' with a comment apologising for it to the static analyser. Click marks a leaf command done on a normal callback return and sets exit code 0, so the gymnastics are not needed.
Hoeze
force-pushed
the
fix/move-populate-to-grz-db
branch
3 times, most recently
from
fcc49c7 to
67a044e
Compare
Four related changes to the populate-side public surface:
- Split 'force' from 'on_missing' on SubmissionDb.populate. 'force'
previously double-duty'd as 'allow destructive overwrites' AND
'silently treat a missing submission as fresh-create'. Add explicit
'on_missing: Literal["create", "error"]' defaulting to 'error'
(raises SubmissionNotFoundError, matching grzctl 'db submission
populate'). The download path opts into 'create' explicitly.
- Drop the now-redundant implicit 'force=True' after auto-create.
Verified: 'Diff.classify' produces only NEW states when old_value is
None, so 'has_pending_destructive' is always False for a fresh row.
- Drop the 'log: logging.Logger | None' parameter. Use the module
logger throughout. Idiomatic Python; callers configure via
'logging.getLogger("grz_db.models.submission")'.
- Drop Worker.load_metadata_with_submission_date. It bundled an S3
head_object and a local file read into one tuple-returning method
to keep the download.py call site short, but those are two distinct
concerns. download.py now calls 'get_metadata_upload_timestamp' and
'worker_inst.parse_submission().metadata.content' separately.
Hoeze
force-pushed
the
fix/move-populate-to-grz-db
branch
from
67a044e to
385b287
Compare
tedil
approved these changes
May 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
grz-common/workers/worker.pyimportedgrz_dbat module top withoutdeclaring it as a dependency, breaking isolated installs of
grz-cli.The fix in the title moves the offending code into
grz-db.The follow-up commits consolidate the resulting populate API and clean
up a few unrelated smells in
grzctlencountered along the way.feat(grz-db,grz-common): clean up SubmissionDb.populate API surface
refactor(grz-db,grz-common,grzctl): extract shared metadata + redaction helpers
fix(grzctl): correct stale grz-cli hint in submission-not-found errors