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?
Jim Fulton wrote: ObjectReader. load_persistent is not a persistent class.
>> Each of these (I think) independently ensures that the "obj" in the
>> "obj._p_changed = None" line in
>> ZODB.serialize.
>
> 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 Persistent
name if you care), whose default implementation in 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): Manager says that
self._p_ jar.setstate( self)
# NOTE: The documentation for IPersistentData
# 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...
_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):
self._p_ deactivate( )
# WARNING, this reintroduces bug 185066 for this class!!!
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