Use publishing context manager. #35
Conversation
| for content in publication.repository_version.content: | ||
| for content_artifact in content.contentartifact_set.all(): | ||
| artifact = _find_artifact(content_artifact, publication.repository_version.repository) | ||
| artifact = find_artifact() |
There was a problem hiding this comment.
I think it would be easier to read if we pass content_artifact, repository as arguments instead of depending on the scope/state.
| with Publication.create(repository_version, publisher) as publication: | ||
| manifest = Manifest('PULP_MANIFEST') | ||
| manifest.write(_publish(publication)) | ||
| metadata = PublishedMetadata( |
There was a problem hiding this comment.
I had originally hoped this would be accomplished by publication.add_metadata(manifest.path). I don't think its really necessary for the plugin writer to know about and use the PublishedMetadata object, but leaving like this is probably more appropriate since the publication is a django model.
| @@ -48,9 +46,16 @@ def _publish(publication): | |||
| Yields: | |||
| Entry: The manifest entry. | |||
| """ | |||
| def find_artifact(): | |||
There was a problem hiding this comment.
IMO, this function doesn't really need a docstring for the file plugin, but it would be helpful for plugin writers who are using the file plugin as an example. If the plugin writer isn't already totally familiar with deferred downloading, this step could be surprising. (Especially because deferred downloading isn't well documented in Pulp 3 yet.)
https://pulp.plan.io/issues/3295