[Pulp-dev] avoiding circular imports in Pulp 3 plugins

[Michael Hrivnak]

Would anyone like to take a shot at answering this question about how these
new classes will be discovered?

https://pulp.plan.io/issues/3074#note-3

Thanks!

On Mon, Oct 16, 2017 at 9:44 PM, Michael Hrivnak <mhrivnak at redhat.com>
wrote:

> Following an in-person discussion, I wrote up this issue to reflect what
> we concluded:
>
> https://pulp.plan.io/issues/3074
>
> Please comment there with questions, thoughts and ideas. I'll be mostly
> out of communication the rest of this week, so feel free to improve and
> groom it without me.
>
> Thanks!
>
> On Fri, Sep 8, 2017 at 4:42 PM, Michael Hrivnak <mhrivnak at redhat.com>
> wrote:
>
>> While working with the FileImporter, I discovered an interesting problem
>> with circular imports.
>>
>> An Importer is both a django Model and has a "sync" method, as shown here:
>>
>> https://github.com/pulp/pulp_file/blob/d684f75e/pulp_file/ap
>> p/models.py#L69
>>
>> We decided to design it this way for simplicity, so that a plugin writer
>> would have a minimum of objects to subclass. It also reduces the number of
>> objects for the core to discover. The Importer would have methods for sync,
>> copy, upload, etc. This is very similar to Pulp 2. And just like in Pulp 2,
>> we know that plugins will often want to implement much of the sync logic in
>> one or more separate python modules.
>>
>> When I tried to move the FileImporter's sync code to a separate module, I
>> quickly found a circular import problem. The FileImporter wants to
>> instantiate and use a Synchronizer. The Synchronizer of course needs to use
>> the data model, both the FileContent model and the corresponding Key
>> namedtuple. A plugin with more types would have yet more models.
>>
>> Thinking of the idea that circular imports often indicate a design
>> problem, there is a single and specific design choice throwing a wrench in
>> the gears. We have a single class acting as both model and controller: the
>> FileImporter. It both defines part of the data model, AND it is expected to
>> perform an action (sync) that makes use of several different models
>> (classic controller territory, and definitely not in-line with OOP).
>>
>> We made the choice deliberately to put the plugin API methods (sync,
>> upload, etc.) on the Importer to give the plugin writer a simple
>> experience. But here we see that is already causing difficulty for plugin
>> writing.
>>
>> Perhaps we should re-consider having the Importer both act as model and
>> controller. I'm using those terms loosely, not to imply that we should have
>> a strict MVC pattern or anything like that, but just to describe what type
>> of role any given portion of code is expected to play.
>>
>> Options
>> ----------
>>
>> We could recommend something akin to dependency injection. When the sync
>> method on an Importer calls some other code, it would need to pass all
>> other model classes needed by that code as arguments. It's fairly simple,
>> but maybe more awkward of a pattern than we want to impose on plugin
>> writers.
>>
>> We could consider the Importer model subclasses to only be a way to store
>> configuration, and keep sync/copy/etc code separate. This would require us
>> to provide a way to make those other things discoverable. It's a clean
>> pattern that would do a much better job of following OOP, and it aligns
>> with best practices of creating django model classes, but we'd need to make
>> the discovery aspect easy.
>>
>> We could make the plugin writer do lazy model loading in their sync code.
>> That seems like a pain though.
>>
>> https://docs.djangoproject.com/en/1.11/ref/applications/#dja
>> ngo.apps.apps.get_model
>>
>> Of those three, I like the second the most. It's a familiar programming
>> pattern, and I think we could make the discovery very easy.
>>
>> Have other ideas, or other opinions/variation on the above ideas?
>>
>> --
>>
>> Michael Hrivnak
>>
>> Principal Software Engineer, RHCE
>>
>> Red Hat
>>
>
>
>
> --
>
> Michael Hrivnak
>
> Principal Software Engineer, RHCE
>
> Red Hat
>



-- 

Michael Hrivnak

Principal Software Engineer, RHCE

Red Hat
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20171128/5780ca2a/attachment.htm>