<div dir="ltr">This is great. I had a chance to look at your plugin and am really excited by having chef support in Pulp.<div class="gmail_extra"><br clear="all"><div><div class="m_-8694928175181000560gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><div dir="ltr"><div>Some responses below.</div><div><br></div></div></div></div></div></div></div></div>
<br><div class="gmail_quote">On Tue, May 15, 2018 at 2:31 PM, Simon Baatz <span dir="ltr"><<a href="mailto:gmbnomis@gmail.com" target="_blank">gmbnomis@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">I created the beginnings of a Pulp 3 plugin to manage Chef cookbooks<br>
[1]. Currently, it supports to create repos, create cookbook content<br>
units, and publish repos. A published & distributed repo will offer<br>
a "universe" API endpoint for tools like Berkshelf.<br>
<br>
I did not implement sync yet. I am waiting for "PendingVersion" to be available<br>
first.<br>
<br>
I ran into a couple of problems/uncertainties described below (sorry for the<br>
lengthy mail). I am new to Django, DRF, and, obviously, Pulp 3 so any remarks or<br>
suggestions are welcome:<br>
<br>
- Create Content: The plugin reads as much meta-data as possible from the actual<br>
cookbook Artifact when creating a content unit. The motivation for this is:<br>
<br>
- One doesn't need a special tool to upload content, which makes uploading by e.g.<br>
a CI job easier.<br>
- It ensures consistency between metadata stored in Pulp and the actual<br>
metadata in the cookbook.<br>
<br>
However, this requires to extract a metadata file from a gzipped tar archive.<br>
Content unit creation is synchronous and doing this work in a synchronous call<br>
might not be optimal (we had a discussion in this topic on the pulp-dev<br>
mailing list already).<br></blockquote><div><br></div><div>I agree. Can you make this an async call in the plugin or is there a need to also make this an async call in core/other plugins?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
- Publication/Distribution: The metadata file ("universe") for a published<br>
cookbook repository contains absolute URLs for download (i.e. these point<br>
to published artifacts in a distribution).<br>
<br>
The current publication/distribution concept seems to have the underlying<br>
assumption that a Publication is fully relocatable: PublishedMetadata<br>
artifacts are created by the publishing task and creating a Distribution is a<br>
synchronous call that determines the base path of the published artifacts.<br>
<br>
This causes a problem with said "universe" file. Ideally, it could be<br>
pre-computed (it lists all artifacts in the repo). However, this can't be<br>
done AFAIK since the base path is unknown at publication time and one can't<br>
generate additional metadata artifacts for a specific distribution later.<br>
<br>
The best solution I came up with was to implement a dynamic API. To reduce the<br>
amount of work to be done, the API does a simple string replacement: During<br>
publication, the universe file is pre-computed using placeholders. In the<br>
dynamic API these placeholders are replaced with the actual base URL of the<br>
distribution.<br>
<br>
However, I would prefer not to be forced to implement a dynamic API for static<br>
information. Is there a way to solve this differently?<br></blockquote><div><br></div><div>Are relative paths an option? If not, I don’t think there’s currently an alternative to a live api. I probably wouldn’t even use a published metadata file TBH. I wonder though if there’s maybe functionality we could add to do this.</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
- Content-Type Header: The "universe" file is JSON and must have a corresponding<br>
"Content-Type" HTTP header.<br>
<br>
However, content type of the development server seems to be "text/html" by<br>
default for all artifacts. Apparently, I can't set the content-type of a<br>
(meta-data) artifact(?)<br></blockquote><div><br></div><div>I think this goes back to not using a published metadata file to serve up your api. However, I could see how it might be useful.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
- Getting the base url of a distribution in the dynamic API is surprisingly<br>
complicated and depends on the inner structure of pulp core (I took the<br>
implementation from 'pulp_ansible'). IMHO, a well defined way to obtain it<br>
should be part of the plugin API.<br></blockquote><div><br></div><div>I agree. Opened: <a href="https://pulp.plan.io/issues/3677" target="_blank">https://pulp.plan.io/<wbr>issues/3677</a></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
- "Content" class: The way to use only a single artifact in Content (like done<br>
in pulp_file) seems to require in-depth knowledge of the<br>
Content/ContentSerializer class and its inner workings.<br>
<br>
The downside of this can already be experienced in the "pulp_file" plugin: The<br>
fields "id" and "created" are missing, since the implementation there just<br>
overrides the 'fields' in the serializer).<br>
<br>
I think two Content types should be part of the plugin API: one with<br>
multiple artifacts, and a simpler one with a single artifact<br></blockquote><div><br></div><div>I began working on this:</div><div><br></div><div><a href="https://github.com/pulp/pulp/pull/3476" target="_blank">https://github.com/pulp/pulp/<wbr>pull/3476</a><br></div><div><br></div><div>But there was opposition around having the Content model be responsible for generating the relative path on the Content Artifact. I’ve opened an issue to see if there’s another way to do this (e.g. using serializers):</div><div><br></div><div><a href="https://pulp.plan.io/issues/3678" target="_blank">https://pulp.plan.io/issues/<wbr>3678</a><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
- Uploading an Artifact that already exists returns an error, which is<br>
annoying if you use http/curl to import artifacts. Suppose some other user<br>
uploaded an artifact in the past. You won't get useful<br>
information from the POST request uploading the same artifact:<br>
<br>
HTTP/1.1 400 Bad Request<br>
Allow: GET, POST, HEAD, OPTIONS<br>
Content-Type: application/json<br>
Date: Sat, 12 May 2018 17:50:54 GMT<br>
Server: WSGIServer/0.2 CPython/3.6.2<br>
Vary: Accept<br>
<br>
{<br>
"non_field_errors": [<br>
"sha512 checksum must be unique."<br>
]<br>
}<br>
<br>
This forced me to do something like:<br>
<br>
...<br>
sha256=$(sha256sum "$targz" | awk '{print $1}')<br>
ARTIFACT_HREF=$(http :8000/pulp/api/v3/artifacts/?s<wbr>ha256=$sha256 | jq -r '.results[0]._href')<br>
if [[ $ARTIFACT_HREF == "null" ]]; then<br>
echo uploading artifact $cookbook_name sha256: $sha256<br>
http --form POST <a href="http://localhost:8000/pulp/api/v3/artifacts/" rel="noreferrer" target="_blank">http://localhost:8000/pulp/api<wbr>/v3/artifacts/</a> file@$targz<br>
ARTIFACT_HREF=$(http :8000/pulp/api/v3/artifacts/?s<wbr>ha256=$sha256 | jq -r '.results[0]._href')<br>
...<br>
<br>
Perhaps a "303 See Other" to the existing artifact would help here.<br>
<br>
<br></blockquote><div><br></div><div>Why not just do something like:</div><div><br></div><div>http --form POST <a href="http://localhost:8000/pulp/api/v3/artifacts/" rel="noreferrer" target="_blank">http://localhost:8000/<wbr>pulp/api/v3/artifacts/</a> file@$<wbr>targz || true<br>ARTIFACT_HREF=$(http :8000/pulp/api/v3/artifacts/?s<wbr>ha256=$sha256 | jq -r '.results[0]._href’)<br></div><div>if [[ $ARTIFACT_HREF == “null” ]]; then exit 1; fi<br></div><div><br></div><div>The error message could be more helpful though. It should probably contain the existing artifact’s href. I looked at 303 and am a bit ambivalent toward using it.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
[1]: <a href="https://github.com/gmbnomis/pulp_cookbook" rel="noreferrer" target="_blank">https://github.com/gmbnomis/pu<wbr>lp_cookbook</a><br>
<br>
______________________________<wbr>_________________<br>
Pulp-dev mailing list<br>
<a href="mailto:Pulp-dev@redhat.com" target="_blank">Pulp-dev@redhat.com</a><br>
<a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman<wbr>/listinfo/pulp-dev</a><br>
</blockquote></div><br></div></div>