Support qualifier-first versions like final-1.2.3 during migration #301
Conversation
Problem: Passing a version like final-1.2.3 results in a 500 error. This happens because the corresponding database field is required and ends up being NULL after parsing fails. Impact: Some repositories (e.g., Artifactory) allow such version formats. As a result, migrating from those repositories to Pulp is currently difficult or impossible. Proposal: Allow versions in the form final-1.2.3 to be recognized as valid.
mdellweg
left a comment
mdellweg
left a comment
There was a problem hiding this comment.
Thanks for bringing this to our attention.
| @@ -0,0 +1 @@ | |||
| Fixed bug related to qualifier-first versions like final-1.2.3 No newline at end of file | |||
There was a problem hiding this comment.
| Fixed bug related to qualifier-first versions like final-1.2.3 | |
| Fixed bug related to qualifier-first versions like `final-1.2.3`. | |
Maybe give the user more details:
| Fixed bug related to qualifier-first versions like final-1.2.3 | |
| Fixed bug related to qualifier-first versions, so e.g. `final-1.2.3` is considered valid now. |
| @@ -1,9 +1,99 @@ | |||
| from django.test import TestCase | |||
There was a problem hiding this comment.
We use pytest to run these tests, and pytest-django is available.
Any chance I could ask you to redesign these around the and db, monkeypatch fixtures and the pytest.mark.parametrize decorator?
There was a problem hiding this comment.
If you want, you can also just test the regex in complete isolation.
| sub_path, filename = path.split(relative_path) | ||
| sub_path, version = path.split(sub_path) | ||
| pattern = re.compile(r"\d+(\.\d+)?(\.\d+)?([.-][a-zA-Z0-9]+)*") | ||
| pattern = re.compile(r"^((?:[A-Za-z0-9]+[.-])+\d+(?:\.\d+){0,2}|\d+(?:\.\d+){0,2}(?:[.-][A-Za-z0-9]+)*)$") |
There was a problem hiding this comment.
Would 1.2.3.4.5.6 be a valid version?
Can you add some negative test cases too?
Is there an official document about maven versioning we can link here?
There was a problem hiding this comment.
@mdellweg @bvladimir Maybe we should remove version regexp-ing at all?
Reasons for do so:
- Apache Maven docs says: "We recommend that the version follow the rules of Semantic Versioning 1.0.0."
- Maven Central Repository docs says: "For version we suggest the usage of semantic version numbers to convey a meaningful number to your users and allow an understandable order of versions. However there is no specific requirement for your version number."
So, if some package in most well-known repo https://repo1.maven.org/maven2/ has "irregular" version - it could fail to load to Pulp...
3 participants
Problem:
Passing a version like final-1.2.3 results in a 500 error. This happens because the corresponding database field is required and ends up being NULL after parsing fails.
Impact:
Some repositories (e.g., Artifactory) allow such version formats. As a result, migrating from those repositories to Pulp is currently difficult or impossible.
Proposal:
Allow versions in the form final-1.2.3 to be recognized as valid.
#300