-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor ArtefactMetadata
model classes
#971
Conversation
4e7c5ff
to
2bd3fbe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
2bd3fbe
to
43c9fcd
Compare
) -> str: | ||
''' | ||
generate stable representation of `artefact_extra_id` and remove `version` key if | ||
the specified version is identical to the given artefact version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why special-handling for version? how about adding this to gci.componentmodel instead, like so:
-> id_tuple: returns (alphabetically) ordered tuple of (, )-pairs, which will always contain as many id-attrs as are available (always name
, version
if available, every attr from extra-id)?
this function's output might be used to also normalise into a str (we might use tuple's str for that or invent a custom format, as you suggested). however, as separation character, I would suggest to use one that is forbidden within keys/values - did you check ocm-spec whether this is true for underscore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We added the special-handling for the version already a while ago (ref). The reason behind this was that some artefacts contained their version also in the extra identity field, however, not always stable across different OCI repositories. Because of that, the comparison of the extra identities failed sometimes, depending of which OCI repository was used. To allow a stable representation of the extra identity independent of the OCI repository, we decided to allow the removal of the version attribute from the extra identity if desired.
Also, I just moved the normalise_artefact_extra_id
function from delivery.model
to dso.model
, so I did not look into it in more detail, e.g. the separating characters. Nevertheless, I figure the underscore character is very basic in Python variable names and thus not forbidden within keys/values.
43c9fcd
to
bada4e3
Compare
bada4e3
to
1c8d4ed
Compare
1c8d4ed
to
65bbfd5
Compare
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Release note: