Comment 8 for bug 185066

Jim Fulton wrote:
>> Each of these (I think) independently ensures that the "obj" in the
>> "obj._p_changed = None" line in
>> ZODB.serialize.ObjectReader.load_persistent is not a persistent class.
>
> a. That probably has the same bug.

I don't understand what you are saying here...

> b. You also need to worry about Connection.get, which also creates ghosts.

Correct. I hadn't noticed that one.

> PersistentClasses are just one example. The concept of
> non-ghostifiable objects is broader.

I understand the use for objects that *once fully loaded* will not again
become a ghost. What you claim is a use for objects that skips the
ghost state entirely and always perform a full load. Can you point me
to an example or a discussion of that use case? I am guessing that
PersistentClasses is one such example, but don't (yet) understand why.

> ...
>
> What's probably needed is a way to mark an object as being a ghost
> independent of _p_deactivate or _p_invalidate. This indicates an
> additional API or possibly meta-class tricks.
>

How about a new method "_p_initghost" (feel free to suggest a better
name if you care), whose default implementation in persistent.Persistent
would be equivalent to the current _p_deactivate. Objects that want to
avoid the ghost state can reimplement it to load the correct state
instead, e.g.:

   def _p_initghost(self):
       # NOTE: The documentation for IPersistentDataManager says that
       # setstate should only be called for objects in the ghost state,
       # but it seems to work anyway. My guess is that it is just a
       # warning against throwing away the current data, which should
       # be OK here because we don't *have* any current data. If I'm
       # wrong, we need another way to load the data...
       self._p_jar.setstate(self)

_p_initghost has well-defined input (whatever klass.__new__(klass, ...)
returns, with _p_oid and _p_jar assigned) and output (either a proper
ghost or a fully loaded object). For objects without a _p_initghost
method, we can fall back to _p_deactivate:

   obj._p_oid = oid
   obj._p_jar = jar
   try:
       meth = obj._p_initghost
   except AttributeError:
       # BBB for non-instances of Persistent
       meth = obj._p_deactivate
   meth()

Code that relies on the current behavior of calling a custom
_p_deactivate while creating a ghost can be made to work in the same in
old and new versions by adding a different override:

   def _p_initghost(self):
       # WARNING, this reintroduces bug 185066 for this class!!!
       self._p_deactivate()

The _p_initghost method could also be defined to take the oid and jar as
arguments and set them on the object. This would allow some additional
error checking in _p_initghost (the call is only valid if _p_oid and
_p_jar are None), but would also add a small extra burden on all
implementations.

> ...
>
>> I agree that _p_deactivate and _p_invalidate are probably entangled a
>> bit too much atm, but I see that as a separate issue.
>
> It's a separate issue if we think that invalidation isn't the right
> model for initializing ghosts.

I don't think invalidation is the right model, because _p_invalidate
should only be called when the object is either a ghost or fully loaded,
which it can't be if _p_invalidate is the method that is supposed to
*make* it either a ghost or fully loaded...

>
>> Hopefully we can
>> get the current one fixed before 3.9.
>
> No, there are too many issues that need to be worked out and too much
> opportunity for breakage.

I see. Just curious, what is the expected timeline for 3.10?

Jacob