<div dir="ltr">Following an in-person discussion, I wrote up this issue to reflect what we concluded:<div><br></div><div><a href="https://pulp.plan.io/issues/3074">https://pulp.plan.io/issues/3074</a><br></div><div><br></div><div>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.</div><div><br></div><div>Thanks!</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Sep 8, 2017 at 4:42 PM, Michael Hrivnak <span dir="ltr"><<a href="mailto:mhrivnak@redhat.com" target="_blank">mhrivnak@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>While working with the FileImporter, I discovered an interesting problem with circular imports.</div><div><br></div><div>An Importer is both a django Model and has a "sync" method, as shown here:</div><div><br></div><div><a href="https://github.com/pulp/pulp_file/blob/d684f75e/pulp_file/app/models.py#L69" target="_blank">https://github.com/pulp/pulp_<wbr>file/blob/d684f75e/pulp_file/<wbr>app/models.py#L69</a><br></div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>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).</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>Options</div><div>----------</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>We could make the plugin writer do lazy model loading in their sync code. That seems like a pain though.</div><div><br></div><div><a href="https://docs.djangoproject.com/en/1.11/ref/applications/#django.apps.apps.get_model" target="_blank">https://docs.djangoproject.<wbr>com/en/1.11/ref/applications/#<wbr>django.apps.apps.get_model</a><br></div><div><br></div><div>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.<br></div><div><br></div><div>Have other ideas, or other opinions/variation on the above ideas?</div><span class="HOEnZb"><font color="#888888"><div><br></div>-- <br><div class="m_7156620837945994225gmail_signature"><div dir="ltr"><p style="color:rgb(0,0,0);font-family:overpass-mono,monospace;font-size:10px;margin:0px;padding:0px"><span style="margin:0px;padding:0px">Michael</span> <span style="margin:0px;padding:0px">Hrivnak</span></p><p style="color:rgb(0,0,0);font-family:overpass-mono,monospace;font-size:10px;margin:0px;padding:0px"></p><span style="color:rgb(0,0,0);font-family:overpass-mono,monospace;font-size:10px;margin:0px;padding:0px"><span style="margin:0px;padding:0px">Principal Software Engineer</span><span style="margin:0px;padding:0px">, <span style="margin:0px;padding:0px">RHCE</span></span> </span><span style="color:rgb(0,0,0);font-family:overpass-mono,monospace;font-size:10px"></span><br style="color:rgb(0,0,0);font-family:overpass-mono,monospace;font-size:10px;margin:0px;padding:0px"><p style="color:rgb(0,0,0);font-family:overpass-mono,monospace;font-size:10px;margin:0px;padding:0px">Red Hat</p></div></div>
</font></span></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><p style="color:rgb(0,0,0);font-family:overpass-mono,monospace;font-size:10px;margin:0px!important;padding:0px!important"><span style="margin:0px!important;padding:0px!important">Michael</span> <span style="margin:0px!important;padding:0px!important">Hrivnak</span></p><p style="color:rgb(0,0,0);font-family:overpass-mono,monospace;font-size:10px;margin:0px!important;padding:0px!important"></p><span style="color:rgb(0,0,0);font-family:overpass-mono,monospace;font-size:10px;margin:0px!important;padding:0px!important"><span style="margin:0px!important;padding:0px!important">Principal Software Engineer</span><span style="margin:0px!important;padding:0px!important">, <span style="margin:0px!important;padding:0px!important">RHCE</span></span> </span><span style="color:rgb(0,0,0);font-family:overpass-mono,monospace;font-size:10px"></span><br style="color:rgb(0,0,0);font-family:overpass-mono,monospace;font-size:10px;margin:0px!important;padding:0px!important"><p style="color:rgb(0,0,0);font-family:overpass-mono,monospace;font-size:10px;margin:0px!important;padding:0px!important">Red Hat</p></div></div>
</div>