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 <[email protected]> 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 <[email protected]> > 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
_______________________________________________ Pulp-dev mailing list [email protected] https://www.redhat.com/mailman/listinfo/pulp-dev
