<div dir="ltr"><div><div>+1 to a guard feature that warns when there are content types in the db that are not present with entry points. I filed that here: <a href="https://pulp.plan.io/issues/2779">https://pulp.plan.io/issues/2779</a> </div>I think there is still an improvement to be made with cast() when it fails to cast. The cast method itself can't know if you're dealing with the master or most detailed object, but the code author calling cast() always knows and if they don't need it cast() they shouldn't call cast(). For example the publish code here [0]. The publish code calls cast() because it needs the type to be the detail version of models.Publisher. Anytime the plugin writer (or core) looks up a master instance and calls cast() and it returns the exact same object that was requested to be cast() that should cause an error instead of failing silently. What do you all think about this? </div><div>What I'm thinking of is something like this hastily-written diff which is similar to what @dkliban mentioned in his email. </div><div> diff --git a/platform/pulp/app/models/base.py b/platform/pulp/app/models/base.py index ead4301..c9248fc 100644 --- a/platform/pulp/app/models/base.py +++ b/platform/pulp/app/models/base.py @@ -90,7 +90,7 @@ class MasterModel(Model): self.type = self.TYPE return super(MasterModel, self).save(*args, **kwargs) - def cast(self): + def cast(self, depth=0): """Return a "Detail" model instance of this master-detail pair. If this model is already an instance of its detail type, it will return itself. @@ -104,12 +104,14 @@ class MasterModel(Model): # related detail table. Cast and resturn this value, recursively following # master/detail relationships down to the last table (the most detailed). try: - return getattr(self, <a href="http://rel.name">rel.name</a>).cast() + return getattr(self, <a href="http://rel.name">rel.name</a>).cast(depth + 1) except AttributeError: continue else: # The for loop exited normally, there are no more detailed models than this # one in this instance's master/detail ancestry, so return here. + if depth == 0: + raise PulpCodedException('Could not cast to detail()') return self @property [0]: <a href="https://github.com/pulp/pulp/commit/bf7433236c5cf5203808341c74715ad257d69718#diff-5e2d175476716cc65d5d2d61b7eb5efdR35">https://github.com/pulp/pulp/commit/bf7433236c5cf5203808341c74715ad257d69718#diff-5e2d175476716cc65d5d2d61b7eb5efdR35</a> </div> </div><div class="gmail_extra"> <div class="gmail_quote">On Fri, May 26, 2017 at 5:15 PM, Sean Myers <<a href="mailto:sean.myers@redhat.com" target="_blank">sean.myers@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 05/26/2017 11:41 AM, Dennis Kliban wrote: > Looking at the cast() method[0] it looks like it's possible to call cast() > on a detail model. I would like to figure out when we expect to call cast() > on a detail model. Without fully knowing the motivation for this > implementation, I am inclined to raise an exception when the code reaches > line 113. The exception would inform the developer that calling cast() is > only appropriate on a master model. You don't always know if you're dealing with a master model instance or its related cast detail model instance. This method makes calling cast on a detail instance a no-op, so it costs when you need to pay the price, and is free when you don't. It also has the effect that "instance is instance.cast()" when casting detail instances. As Michael pointed out, raising an exception here would be an expection antipattern (raising exceptions in non-exceptional cases). On 05/26/2017 04:27 PM, Dennis Kliban wrote: > The problem occurs when a worker receives a task to perform a publish, but > it doesn't have the plugin installed. The problem is that the plugin is not installed on the worker, so fix *that* problem. Install the plugin on the worker. If anything should raise an exception, it should be a warning on worker start that unknown content types exist in the db, including what the master type is and what the unknown content types are. _______________________________________________ Pulp-dev mailing list <a href="mailto:Pulp-dev@redhat.com">Pulp-dev@redhat.com</a> <a href="https://www.redhat.com/mailman/listinfo/pulp-dev" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/pulp-dev</a> </blockquote></div> </div>