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

[Dennis Kliban]

I agree with everything you said.

The problem occurs when a worker receives a task to perform a publish, but
it doesn't have the plugin installed. As a result the call to
publisher.cast() returns the master model. The publish task then tries to
call a method on the master model that does not exist. The user then sees
an attribute not found exception in the logs. It would be better to raise a
more useful exception in the cast() method itself when it is not able to
cast to anything.

One suggestion was to add an optional call_counter keyword argument to
cast(self, call_count=0). The argument would only be passed in to
cast(call_count=call_count+1) when it is being recursively called. Then we
could check if line 113 is reached with call_count==0 and raise an
exception saying that the master model could not be cast to a detail model.

Thoughts?

On Fri, May 26, 2017 at 1:23 PM, Michael Hrivnak <mhrivnak at redhat.com>
wrote:

> Interesting question. It looks like in this implementation, even if you
> call cast() on a master model, the method itself will kind-of-recursively
> call cast() on detail models until it gets to the most detailed one, which
> will return itself. So every time cast() is called, eventually the most
> detailed model is expected to have its cast() method called and must return
> itself.
>
> We could add a special case where the last one raises an exception, and
> the next-to-last one catches it, but I'm not sure that extra complication
> would be worth it. We'd be making the most common case "exceptional".
>
> Having the call be idempotent is also potentially a perk, depending on how
> you look at it. Based on all that, plus the doc block confirming that the
> behavior is intentional, I don't see a problem with the current behavior.
>
> On Fri, May 26, 2017 at 11:41 AM, Dennis Kliban <dkliban at redhat.com>
> 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. What are your thoughts?
>>
>>
>> [0] https://github.com/pulp/pulp/blob/3.0-dev/platform/pulp/app/
>> models/base.py#L113
>>
>>
>> -Dennis
>>
>> _______________________________________________
>> Pulp-dev mailing list
>> Pulp-dev at redhat.com
>> https://www.redhat.com/mailman/listinfo/pulp-dev
>>
>>
>
>
> --
>
> Michael Hrivnak
>
> Principal Software Engineer, RHCE
>
> Red Hat
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/pulp-dev/attachments/20170526/969abd59/attachment.htm>