Fixes #33221 - Hide upload section for Collection repository #9540
Conversation
|
Issues: #33221 |
sjha4
changed the title
|
Opened issue: https://pulp.plan.io/issues/9220 in pulp to enable easier upload of collections without the several user specified required fields. |
There was a problem hiding this comment.
Looks good overall. Can't see the upload section!
One potential issue I notice is I can cause an error by making an API request to create an upload request. This is the create method in content_uploads_controller.rb. Might not be an actual use-case though.
Request:
curl --user admin:changeme --header "Content-Type: application/json" --request POST --data '{"size":"8000","content_type":"ansible collection"}' https://centos7-katello-devel-nfs.localhost.example.com/katello/api/repositories/2/content_uploads.
Also, ansible collections aren't marked as uploaded (RepositoryTypeManager.uploadable_content_types). I wonder if there's a way to check that rather than block ansible repositories entirely? That might require changes that are out of scope for this card though, because content_type isn't a required param.
|
@rverdile : Let me update the content_upload controller. The upload itslef won't work but we should probably fail graciously. The current code will throw a null reference. |
sjha4
force-pushed
the
hide_upload_ansible_collections
branch
from
4726f88
to
0960aea
Compare
| @@ -13,6 +13,8 @@ class Api::V2::ContentUploadsController < Api::V2::ApiController | |||
| param :content_type, RepositoryTypeManager.uploadable_content_types(false).map(&:label), :required => false, :desc => N_("content type ('deb', 'docker_manifest', 'file', 'ostree', 'rpm', 'srpm')") | |||
| def create | |||
| content_type = params[:content_type] || ::Katello::RepositoryTypeManager.find(@repository.content_type).default_managed_content_type.label | |||
| fail _("Content type is not supported for upload!") unless content_type | |||
There was a problem hiding this comment.
I can cause two different errors here.
The first one is if I don't specify content_type in the API request, the return message is "undefined method label for nil:NilClass","errors":["undefined method label for nil:NilClass"].
The second one is if I pass ansible collections as content_type, the return message is "uninitialized constant Katello::Pulp3::AnsibleCollection::CONTENT_TYPE","errors":["uninitialized constant Katello::Pulp3::AnsibleCollection::CONTENT_TYPE"]
But I think the solution you used for repositories_controller would work fine here.
There was a problem hiding this comment.
Can you check these on the updated commit..
I am a little surprised you were able to pass ansible_collections as a param and not fail at the param validation itself or maybe I am reading it wrong:
param :content_type, RepositoryTypeManager.uploadable_content_types(false).map(&:label), :required => false, :desc => N_("content type ('deb', 'docker_manifest', 'file', 'ostree', 'rpm', 'srpm')")
RepositoryTypeManager.uploadable_content_types(false).map(&:label) does not return ansible_collections so it should ideally fail before going any further. In any case, I added that check inside the controller to prevent that.
sjha4
force-pushed
the
hide_upload_ansible_collections
branch
3 times, most recently
from
1d9a547
to
56cdd38
Compare
sjha4
force-pushed
the
hide_upload_ansible_collections
branch
from
56cdd38
to
3e24128
Compare
(cherry picked from commit 5ad117c)
Was looking into the uploads flow for collections and seems like it has to be a workflow of sorts to have these fields: https://docs.pulpproject.org/pulp_ansible/restapi.html#operation/content_ansible_collection_versions_create
For the purposes of current release, hiding the upload section on repo details and adding a user friendly error to the API.