Closed icemac closed 8 months ago
I have no idea what I made wrong here, so I'll stop working on this PR.
@icemac I fixed the segfault. PyObject_GetOptionalAttr expects the attribute name as a Python string, not a _PY_IDENTIFIER. The fixed implementation is based on the implementation of _PyObject_LookupAttrId that was removed from CPython.
@davisagli Thank you for making the build green.
After this PR is merged and released https://github.com/zopefoundation/zope.container/pull/53 could turn green.
David Glick wrote at 2024-2-14 00:04 -0800:
... @d-maurer @icemac I don't know the consequences of ignoring all exceptions vs only AttributeError, so I'd prefer to keep the implementation as it is.
In general, it is a bad idea to use hasattr
(i.e. to ignore all
exceptions) on a persistent object:
the access could cause a ConflictError
which hasattr
would turn
into False
even though the object might have that attribute but the
object is not loadable at the moment. Instead of returning False
,
the request should get retried in that case.
I assume that even though zodbpickle
works with persistent objects,
the function at hand will be called only for special
methods (__getstate__
, __setstate__
, ...) which are not called
on the object itself but on its class.
Therefore, hasattr
might be safe, but I do not mind to
retain the current implementation.
Thank you for reviewing resp. working on this PR. 😃
Open issues