Add PRN support #5813
Add PRN support #5813
Conversation
gerrod3
force-pushed
the
prn-all
branch
4 times, most recently
from
463dd68
to
af4fcce
Compare
gerrod3
force-pushed
the
prn-all
branch
2 times, most recently
from
e5bbdee
to
cf9fc98
Compare
| ) | ||
| repo_versions[href_match["repository_pk"]].append(int(href_match["number"])) | ||
| if uri.startswith("prn:"): | ||
| model, pk = resolve_prn(uri) |
There was a problem hiding this comment.
So a repository version PRN is not prn:file.repository:<uuid>:<version> ?
There was a problem hiding this comment.
Nope, it is always prn:<app_label>.<model_label>:<uuid>, even for repository versions.
| assert ctx.value.status == 404 | ||
| assert ctx.value.status == 400 |
There was a problem hiding this comment.
Hmmm. Was it wrong raising 404 before?
There was a problem hiding this comment.
It kind of makes sense, you couldn't find a badly named resource so 404 not found. But it's an improper input so it should probably return 400, which it now does since I changed the filter code.
gerrod3
force-pushed
the
prn-all
branch
2 times, most recently
from
5314e51
to
ef97427
Compare
| if name.endswith("Response"): | ||
| if "pulp_href" in schema["properties"]: | ||
| assert "prn" in schema["properties"], name | ||
| assert schema["properties"]["prn"]["type"] == "string", name |
There was a problem hiding this comment.
This fails, right now, if you have pulp_rpm installed with "rpm.DistributionTreeResponse". I'm sure there are more. Can we set this up to collect/log all the failures, and then fail the test?
There was a problem hiding this comment.
Ahh, I didn't test with other plugins installed.
There was a problem hiding this comment.
A quick "manual" scan shows pulp_rpm is missing the base fields in DistributionTreeSerializer and UpdateCollectionSerializer, fwiw. Would be good to have the test give us a complete list for a given install, so a run with many-plugins would ferret out the list of serializers that need to be fixed.
There was a problem hiding this comment.
Ok, I've updated the test to group them together. Can you try it out and see if it is correct?
There was a problem hiding this comment.
Ha! OK, it works - but it still only finds DistributionTreeSerializer - because UpdateCollectionSerializer doesn't expose pulp_href!
> assert len(failed) == 0
E AssertionError: assert 1 == 0
E + where 1 = len(['rpm.DistributionTreeResponse'])Let me reset my env to include container and deb and let's see what we get, give me a bit.
There was a problem hiding this comment.
OK cool, this looks like it does pick up all the ones it finds:
E AssertionError: assert 5 == 0
E + where 5 = len(['CollectionSummaryResponse', 'ansible.AnsibleDistributionResponse', 'ansible.AnsibleNamespaceMetadataResponse', 'ansible.CollectionVersionMarkResponse', 'rpm.DistributionTreeResponse'])(That's from an env with DEV_SOURCE_PATH=pulpcore:pulp_rpm:pulp_container:pulp_python:pulp_deb:pulp_ansible , btw)
There was a problem hiding this comment.
We used to have a set of tests in pulpcore that will run on the plugins ci. I'm not sure this is still a thing. But this sounds like a good candidate.
fixes: pulp#5766
fixes: #5766