Serialize AdvisoryPackage.package_name in v3 JSON - #95
Open
atomicturtle wants to merge 3 commits into
Open
Conversation
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
requested changes
Sep 1, 2026
rockythorn
left a comment
Collaborator
There was a problem hiding this comment.
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:1444now returnspackage_name: "nodejs"instead ofnullmodule.prefix is correctly stripped (nomodule.-prefixed names in output)- No null
package_namevalues 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_nameconverted to@staticmethod— good, no need forselfreplace('module.', '', 1)— fixes a real edge case where doublemodule.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:
- Add a
py_testentry toapollo/tests/BUILD.bazel:
py_test(
name = "test_advisory_package",
srcs = ["test_advisory_package.py"],
deps = [
"//apollo/db:db_lib",
],
)- Add to
.github/workflows/test.yamlin 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.
3 tasks
The serializer test existed on the branch but was not wired into BUILD.bazel or the GitHub Actions unit-test list.
Contributor
Author
|
Registered Still just the v3 JSON serialization fix for review — not a production catalog change. |
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
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.
Summary
packages[].package_namewas always null because Tortoise pydantic only emits computed methods, and the cleaned name lived on a property over_package_name.GET /api/v3/advisories/{id}serializespackage_name.Test plan
python -m unittest apollo.tests.test_advisory_packagecurl -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), notnull.Closes #94