Fix stale pull request CI compatibility - #641
Merged
Merged
Conversation
Collaborator
There was a problem hiding this comment.
Pull request overview
This PR restores compatibility for older/in-flight branches and stabilizes API serializer output ordering after the drf-spectacular changes in #628, while also ensuring CI’s mreg-cli integration tests exercise the image built for the current PR.
Changes:
- Make
NetworkSerializerandNetGroupRegexPermissionSerializeruse explicitMeta.fieldstuples to preserve the pre-#628 serialized field order while keeping the explicit OpenAPI-friendly custom fields. - Reintroduce
mreg.api.views.LIBRARIES_TO_REPORTas an alias forREPORTED_LIBRARY_VERSION_FIELDSand add a regression test for the alias. - Fix CI image tagging so the mreg-cli workflow uses the locally built PR image (tagged as
ghcr.io/unioslo/mreg:master) and add regression tests for serializer field ordering.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
mreg/api/v1/serializers.py |
Switch NetworkSerializer and NetGroupRegexPermissionSerializer from __all__ to explicit ordered field tuples to stabilize output order. |
mreg/tests/test_serializers.py |
Add regression tests asserting the serializer field order for the two affected serializers. |
mreg/api/views.py |
Restore LIBRARIES_TO_REPORT as a compatibility alias to the renamed constant in mreg.api.serializers. |
mreg/api/tests/test_versions.py |
Add regression coverage ensuring the legacy constant remains compatible (same object). |
.github/workflows/container-image.yml |
Tag the locally built image as ghcr.io/unioslo/mreg:master so mreg-cli tests the PR artifact instead of a registry image. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
NetworkSerializerandNetGroupRegexPermissionSerializer.networkandrangefields required for OpenAPI generation.mreg.api.views.LIBRARIES_TO_REPORTas a compatibility alias.Root cause
Serializer field ordering
PR #628 explicitly declared two custom model fields so drf-spectacular could describe them:
Combined with
fields = "__all__", DRF moved those fields earlier in the serialized output.NetworkSerializerchanged from:to:
NetGroupRegexPermissionSerializerchanged from:to:
Although JSON object order is not semantically significant, mreg-cli’s recorded-output tests compare it, and stable serialization avoids unnecessary changes for existing consumers.
Both serializers now use explicit field tuples in their previous order while retaining the explicit custom fields required for OpenAPI generation.
Renamed constant
PR #628 moved the reported-library constant from
mreg.api.viewstomreg.api.serializersand renamed it toREPORTED_LIBRARY_VERSION_FIELDS.In-flight pull requests created before #628, including #616, still import
LIBRARIES_TO_REPORTfrommreg.api.views. The previous name is retained as an alias:This keeps older branches compatible without duplicating the value.
Incorrect image used by mreg-cli
The container workflow tagged the newly built PR image locally as:
However, the mreg-cli integration script defaults to:
The integration job could therefore test the published registry image instead of the image built for the current pull request.
The workflow now tags the locally built image as
:master, ensuring mreg-cli exercises the PR artifact. This local tag is not pushed to the registry.