Creates basic pulp.plugin

[bmbouter]

This PR adds a base Cataloger and Profiler objects to
pulp.plugin. It as imports the following objects from
pulp.platform.models for usage by plugins:

• Distributor
• Importer
• Repository
• RepositoryContent

It removes several modules from the old pulp.plugins
package.

This does not have an interface for upload. After thinking about it I believe platform can do everything for upload assuming that a plugin writer has defined the Content type correctly. If this turns out not to be the case we can easily add the upload interface.

closes #2181
https://pulp.plan.io/issues/2181

[mention-bot]

@bmbouter, thanks for your PR! By analyzing the annotation information on this pull request, we identified @jortel, @mhrivnak and @asmacdo to be potential reviewers

[jeremycline]

I think it'd be super neat if we used Spinx-y docstrings where mentions of classes, for example, are written as :class:class.path.Here``. That way we can generate API docs for our public Python interfaces that look like this (source)! It's really handy to have links to the classes and their documentation right there, with syntax-highlighted examples, etc.

[bmbouter]

I'm going to put some work into making the docstrings great since this is the new Plugin API, but having some more comments on the code currently here would be great.

[jortel]

This needs to be from pulp.platform.models import ....

[bmbouter]

good catch, done.

[jortel]

So far, I like this approach the best. It provides clear separation of concerns between the platform (core) model and the plugin integration (layer) model. Plugin (contributed) objects are used differently by the platform so having them be an extension of the platform model instead of part of the platform model seems appropriate. Also, this achieves the goal of simplifying the plugin writer's universe.

It reinforces this:

--------------------------
|         REST           |
--------------------------
|    Platform (core)     |
--------------------------
|   Plugin (integration) |
--------------------------
| p1 | p2 | p3 | p4 | p5 |
|    |    |    |    |    |

I assume you'll be adding additional methods on these classes for feature parity with pulp2 model after we have consensus on the approach?

[bmbouter]

@jortel Thanks a lot for the review. Everything suggested makes sense and I will revise it today along with improving the docstrings.

The one thing I had not planned on doing was adding more methods. The upload in 2.y was on the importer but I think that can become an all-platform activity and the plugin writer would not have to implement anything to support it. Similarly for Copy. It's likely other methods do need to be added though. What do you think those methods are?

[jortel]

Agreed about the upload() and copy().

After review of the pulp2 models only a few things come to mind.

• Do we still need anything like ensure_all_units_downloaded()? I'm hoping not but don't know why it's needed in pulp2. Could be added later if we discover it's needed.
• Do we still need the distributor to participate in clean up after it (distributor) has been deleted (like in pulp2)?
• And, lastly, something new for pulp3. I did not model anything in the platform for share content storage. After much consideration, it seemed more appropriate for the plugin to solely manage that. Given this, the plugin would need to participate in orphan purging. Perhaps the importer? Something like: content_deleted(content). This could be added later. Thoughts?

[mhrivnak]

It would be great to emphasize here that the plugin writer should definitely not try to implement their publish logic on this class. They should implement it somewhere else, and just call their workflow from here.

[mhrivnak]

Looking very good! I think this will be mergeable when your next changes are done.

[mhrivnak]

@jortel some responses:

Do we still need the distributor to participate in clean up after it (distributor) has been deleted (like in pulp2)?

I think removing published data when the distributor gets deleted will continue to be important. There may be a more elegant way to trigger that on the model than how we did it in pulp 2, such as using the post_delete signal.

And, lastly, something new for pulp3. I did not model anything in the platform for share content storage. After much consideration, it seemed more appropriate for the plugin to solely manage that. Given this, the plugin would need to participate in orphan purging. Perhaps the importer? Something like: content_deleted(content). This could be added later. Thoughts?

That sounds like it might be a good fit for a class or static method on the unit model, but I'm light on details of what it would need to do.

[jeremycline]

Do we still need the distributor to participate in clean up after it (distributor) has been deleted (like in pulp2)?

My preference would be to have the platform provide a set of methods to interact with the filesystem which records the ownership of files and purges them (accounting for shared directories) automatically. It doesn't seem like something the plugin writer should ever need to worry about.

We already provide a utility method to attempt to handle race conditions during directory creation, anyway.

[mhrivnak]

Recapping discussion with @bmbouter

Upload could (as one option) be handled by putting a class method on each unit model called something like from_files(cls, path). The platform would enable the user to upload files and associate them with a particular upload operation, then submit/finalize the upload. In that last step, the platform would call the class method on the model.

Copy and remove are both great candidates for a default implementation in the platform, with the option for the plugin writer to override and provide custom logic. Examples for why that's useful include dependency resolution in pulp_rpm, copying docker manifests and blobs along with the tags that reference them, etc.

I agree that all of that can be considered and implemented in the future and not on this PR.

[mhrivnak]

My preference would be to have the platform provide a set of methods to interact with the filesystem which records the ownership of files and purges them (accounting for shared directories) automatically. It doesn't seem like something the plugin writer should ever need to worry about.

We already provide a utility method to attempt to handle race conditions during directory creation, anyway.

Majorly agree. This lines up well with the "managed publications" idea I've described in the past. We'll get there!

[mhrivnak]

I find the first few words a little awkward to read. I think you could just cut them out and reduce this to "The model attributes encapsulate all of the information required."

[bmbouter]

agreed

[mhrivnak]

I made some suggestions, but otherwise it looks fine to me.

[bmbouter]

Thanks @mhrivnak I updated with all those revisions. They were good. I'll wait for questions/ideas or an LGTM from a second reviewer. @pcreech @jortel @seandst

[jortel]

should be :class:, right?

[jortel]

I'm satisfied. The suggestion made by @mhrivnak is good (though, I strongly oppose using signals) that the derived plugin Content class can override delete() and do what is necessary. No plugin API call is needed. As with all models, this will likely need some iteration so good to merge this now.

[pcreech]

Based on everything here, this looks good to me as well