Skip to content

Serialize AdvisoryPackage.package_name in v3 JSON - #95

Open
atomicturtle wants to merge 3 commits into
resf:mainfrom
atomicturtle:fix/15-package-name
Open

Serialize AdvisoryPackage.package_name in v3 JSON#95
atomicturtle wants to merge 3 commits into
resf:mainfrom
atomicturtle:fix/15-package-name

Conversation

@atomicturtle

Copy link
Copy Markdown
Contributor

Summary

  • packages[].package_name was always null because Tortoise pydantic only emits computed methods, and the cleaned name lived on a property over _package_name.
  • Keep attribute access for workers/updateinfo, but expose the cleaned name as a computed field so GET /api/v3/advisories/{id} serializes package_name.

Test plan

  • python -m unittest apollo.tests.test_advisory_package
  • After deploy, curl -s https://apollo.build.resf.org/api/v3/advisories/RLSA-2024:1444 | jq '.packages[0].package_name' is a non-null RPM name (e.g. nodejs), not null.

Closes #94

Tortoise pydantic only emits computed methods, so the package_name
property never appeared in GET /api/v3/advisories and the field was
always null. Expose the cleaned name as a computed field while keeping
attribute access for the rest of the workers.

@rockythorn rockythorn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review: Request changes (one fix needed, then approve)

Tested locally against a prod DB snapshot. The serialization fix works correctly.

Verified

  • GET /api/v3/advisories/RLSA-2024:1444 now returns package_name: "nodejs" instead of null
  • module. prefix is correctly stripped (no module.-prefixed names in output)
  • No null package_name values on packages that have names in the DB
  • The descriptor pattern is well-documented and correctly satisfies Tortoise pydantic's requirement for a callable with return annotation
  • _clean_package_name converted to @staticmethod — good, no need for self
  • replace('module.', '', 1) — fixes a real edge case where double module.module. would be over-stripped

Fix needed

test_advisory_package.py is not registered in BUILD.bazel or .github/workflows/test.yaml. The test file exists but will never run in CI. Per project guidelines:

  1. Add a py_test entry to apollo/tests/BUILD.bazel:
py_test(
    name = "test_advisory_package",
    srcs = ["test_advisory_package.py"],
    deps = [
        "//apollo/db:db_lib",
    ],
)
  1. Add to .github/workflows/test.yaml in the "Unit Tests" section.

Design note (non-blocking)

The _PackageName descriptor is the cleverest code in the model layer. The docstring explains the why well. Future maintainers should know this exists if they ever touch Tortoise pydantic computed fields.

The serializer test existed on the branch but was not wired into
BUILD.bazel or the GitHub Actions unit-test list.
@atomicturtle

Copy link
Copy Markdown
Contributor Author

Registered test_advisory_package in apollo/tests/BUILD.bazel and .github/workflows/test.yaml so CI actually runs it. Also merged current main.

Still just the v3 JSON serialization fix for review — not a production catalog change.

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.

Apollo: packages[].package_name is always null in v3 JSON

2 participants