[Pulp-dev] [pulp 3] cast() method for casting from a Master to Detail model instance

[Brian Bouterse]

+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:
https://pulp.plan.io/issues/2779

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?

What I'm thinking of is something like this hastily-written diff which is
similar to what @dkliban mentioned in his email.

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, rel.name).cast()
+                    return getattr(self, rel.name).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]:
https://github.com/pulp/pulp/commit/bf7433236c5cf5203808341c74715ad257d69718#diff-5e2d175476716cc65d5d2d61b7eb5efdR35



On Fri, May 26, 2017 at 5:15 PM, Sean Myers <sean.myers at redhat.com> wrote:

> 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
> Pulp-dev at redhat.com
> https://www.redhat.com/mailman/listinfo/pulp-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20170529/db9714fe/attachment.htm>