This is an automated email from the ASF dual-hosted git repository.
astitcher pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/qpid-proton.git
The following commit(s) were added to refs/heads/main by this push:
new e366bcde8 PROTON-2868: [Python] Simplify C object wrapping code
e366bcde8 is described below
commit e366bcde8c5347dbfd089b8d5a10a71ed7775c74
Author: Andrew Stitcher <astitc...@apache.org>
AuthorDate: Thu Jan 9 17:26:27 2025 -0500
PROTON-2868: [Python] Simplify C object wrapping code
- Cleanup the Wrapper code so that it uses more current less magical
idiomatic python to wrap the C python objects that need to be passed
via the Proton event queue.
- Use __new__ rather than __init__ because we are dealing with
creating the objects rather than just initialising their values
- Use slots because the actual python objects only have 2 fixed
attributes.
- Instead of passing constructor and get_context functions to the
wrapper these are now declarative class variables defined in the
wrapped classes.
- The wrapped classes can now use __init__ more normally: They do still
have to check whether they are already initialised, and not initialise
again if they are, because they could be wrapping a stored away object
that has already been initialised and used rather than a new wrapping.
- We now provide a general wrap classmethod that can bew used for most
of the wrapped types and avoid duplicating the code.
- Needed to add a custom __new__ to Transport because its constructor
parameters aren't compatible with the Wrapper class.
- Transport still needs its custom wrap method for the same reason.
- The SASL class is no longer a wrapped class (and actually never needed
to be one).
---
python/cproton.py | 18 ++-----
python/proton/_delivery.py | 23 ++++-----
python/proton/_endpoints.py | 64 ++++++++++---------------
python/proton/_transport.py | 46 +++++++++++-------
python/proton/_wrapper.py | 112 ++++++++++++++++++++------------------------
5 files changed, 116 insertions(+), 147 deletions(-)
diff --git a/python/cproton.py b/python/cproton.py
index 61a60589e..528e675b9 100644
--- a/python/cproton.py
+++ b/python/cproton.py
@@ -340,24 +340,14 @@ def pn_collector_put_pyref(collector, obj, etype):
lib.pn_collector_put_py(collector, d, etype.number)
-def pn_record_def_py(record):
- lib.pn_record_def_py(record)
-
-
def pn_record_get_py(record):
d = lib.pn_record_get_py(record)
if d == ffi.NULL:
- return None
- return ffi.from_handle(d)
-
-
-def pn_record_set_py(record, value):
- if value is None:
- d = ffi.NULL
- else:
- d = ffi.new_handle(value)
+ d = ffi.new_handle({})
retained_objects.add(d)
- lib.pn_record_set_py(record, d)
+ lib.pn_record_def_py(record)
+ lib.pn_record_set_py(record, d)
+ return ffi.from_handle(d)
def pn_event_class_name(event):
diff --git a/python/proton/_delivery.py b/python/proton/_delivery.py
index 5d62d86b6..6332fd8a7 100644
--- a/python/proton/_delivery.py
+++ b/python/proton/_delivery.py
@@ -24,11 +24,11 @@ from cproton import PN_ACCEPTED, PN_MODIFIED, PN_RECEIVED,
PN_REJECTED, PN_RELEA
pn_delivery_writable, pn_disposition_annotations,
pn_disposition_condition, pn_disposition_data, \
pn_disposition_get_section_number, pn_disposition_get_section_offset,
pn_disposition_is_failed, \
pn_disposition_is_undeliverable, pn_disposition_set_failed,
pn_disposition_set_section_number, \
- pn_disposition_set_section_offset, pn_disposition_set_undeliverable,
pn_disposition_type, \
- isnull
+ pn_disposition_set_section_offset, pn_disposition_set_undeliverable,
pn_disposition_type
from ._condition import cond2obj, obj2cond
from ._data import dat2obj, obj2dat
+from ._transport import Transport
from ._wrapper import Wrapper
from enum import IntEnum
@@ -39,7 +39,7 @@ if TYPE_CHECKING:
from ._condition import Condition
from ._data import PythonAMQPData, symbol
from ._endpoints import Receiver, Sender # circular import
- from ._reactor import Connection, Session, Transport
+ from ._reactor import Connection, Session
class DispositionType(IntEnum):
@@ -277,19 +277,12 @@ class Delivery(Wrapper):
delivery being settled.
"""
- @staticmethod
- def wrap(impl):
- if isnull(impl):
- return None
- else:
- return Delivery(impl)
+ get_context = pn_delivery_attachments
def __init__(self, impl):
- Wrapper.__init__(self, impl, pn_delivery_attachments)
-
- def _init(self) -> None:
- self.local = Disposition(pn_delivery_local(self._impl), True)
- self.remote = Disposition(pn_delivery_remote(self._impl), False)
+ if self.Uninitialized():
+ self.local = Disposition(pn_delivery_local(self._impl), True)
+ self.remote = Disposition(pn_delivery_remote(self._impl), False)
@property
def tag(self) -> str:
@@ -414,7 +407,7 @@ class Delivery(Wrapper):
return self.session.connection
@property
- def transport(self) -> 'Transport':
+ def transport(self) -> Optional[Transport]:
"""
The :class:`Transport` bound to the :class:`Connection` over which
the delivery was sent or received.
diff --git a/python/proton/_endpoints.py b/python/proton/_endpoints.py
index ae7ab81d6..38dde5e6d 100644
--- a/python/proton/_endpoints.py
+++ b/python/proton/_endpoints.py
@@ -54,8 +54,7 @@ from cproton import PN_CONFIGURATION, PN_COORDINATOR,
PN_DELIVERIES, PN_DIST_MOD
pn_terminus_is_dynamic, pn_terminus_outcomes, pn_terminus_properties,
pn_terminus_set_address, \
pn_terminus_set_distribution_mode, pn_terminus_set_durability,
pn_terminus_set_dynamic, \
pn_terminus_set_expiry_policy, pn_terminus_set_timeout,
pn_terminus_set_type, \
- pn_link_properties, pn_link_remote_properties, \
- isnull
+ pn_link_properties, pn_link_remote_properties
from ._condition import cond2obj, obj2cond
from ._data import Data, dat2obj, obj2dat, PropertyDict, SymbolList
@@ -64,7 +63,7 @@ from ._exceptions import ConnectionException, EXCEPTIONS,
LinkException, Session
from ._handler import Handler
from ._transport import Transport
from ._wrapper import Wrapper
-from typing import Dict, List, Optional, Union, TYPE_CHECKING, Any
+from typing import Any, Dict, List, Optional, Union, TYPE_CHECKING
if TYPE_CHECKING:
from ._condition import Condition
@@ -112,7 +111,7 @@ class Endpoint(object):
REMOTE_CLOSED = PN_REMOTE_CLOSED
""" The remote endpoint state is closed. """
- def _init(self) -> None:
+ def __init__(self) -> None:
self.condition: Optional['Condition'] = None
self._handler: Optional[Handler] = None
@@ -159,26 +158,17 @@ class Connection(Wrapper, Endpoint):
A representation of an AMQP connection.
"""
- @staticmethod
- def wrap(impl):
- if isnull(impl):
- return None
- else:
- return Connection(impl)
+ constructor = pn_connection
+ get_context = pn_connection_attachments
def __init__(self, impl: Any = None) -> None:
- if impl is None:
- Wrapper.__init__(self, constructor=pn_connection,
get_context=pn_connection_attachments)
- else:
- Wrapper.__init__(self, impl, pn_connection_attachments)
-
- def _init(self) -> None:
- Endpoint._init(self)
- self.offered_capabilities_list = None
- self.desired_capabilities_list = None
- self.properties = None
- self.url = None
- self._acceptor = None
+ if self.Uninitialized():
+ Endpoint.__init__(self)
+ self.offered_capabilities_list = None
+ self.desired_capabilities_list = None
+ self.properties = None
+ self.url = None
+ self._acceptor = None
def _get_attachments(self):
return pn_connection_attachments(self._impl)
@@ -550,15 +540,12 @@ class Connection(Wrapper, Endpoint):
class Session(Wrapper, Endpoint):
"""A container of links"""
- @staticmethod
- def wrap(impl):
- if isnull(impl):
- return None
- else:
- return Session(impl)
+
+ get_context = pn_session_attachments
def __init__(self, impl):
- Wrapper.__init__(self, impl, pn_session_attachments)
+ if self.Uninitialized():
+ Endpoint.__init__(self)
def _get_attachments(self):
return pn_session_attachments(self._impl)
@@ -714,21 +701,18 @@ class Link(Wrapper, Endpoint):
RCV_SECOND = PN_RCV_SECOND
"""The receiver will only settle deliveries after the sender settles."""
- @staticmethod
- def wrap(impl):
- if isnull(impl):
- return None
+ get_context = pn_link_attachments
+
+ def __new__(cls, impl) -> 'Link':
if pn_link_is_sender(impl):
- return Sender(impl)
+ return super().__new__(Sender, impl)
else:
- return Receiver(impl)
+ return super().__new__(Receiver, impl)
def __init__(self, impl):
- Wrapper.__init__(self, impl, pn_link_attachments)
-
- def _init(self) -> None:
- Endpoint._init(self)
- self.properties = None
+ if self.Uninitialized():
+ Endpoint.__init__(self)
+ self.properties = None
def _get_attachments(self):
return pn_link_attachments(self._impl)
diff --git a/python/proton/_transport.py b/python/proton/_transport.py
index 489654b14..b1cbfa16e 100644
--- a/python/proton/_transport.py
+++ b/python/proton/_transport.py
@@ -92,27 +92,32 @@ class Transport(Wrapper):
else:
return Transport(impl=impl)
+ constructor = pn_transport
+ get_context = pn_transport_attachments
+
+ def __new__(
+ cls,
+ mode: Optional[int] = None,
+ impl=None,
+ ) -> 'Transport':
+ return super().__new__(cls, impl)
+
def __init__(
self,
- mode: 'Optional[int]' = None,
- impl: 'Callable' = None,
+ mode: Optional[int] = None,
+ impl=None,
) -> None:
- if impl is None:
- Wrapper.__init__(self, constructor=pn_transport,
get_context=pn_transport_attachments)
- else:
- Wrapper.__init__(self, impl, pn_transport_attachments)
if mode == Transport.SERVER:
pn_transport_set_server(self._impl)
elif mode is None or mode == Transport.CLIENT:
pass
else:
raise TransportException("Cannot initialise Transport from mode:
%s" % str(mode))
-
- def _init(self) -> None:
- self._sasl = None
- self._ssl = None
- self._reactor = None
- self._connect_selectable = None
+ if self.Uninitialized():
+ self._sasl = None
+ self._ssl = None
+ self._reactor = None
+ self._connect_selectable = None
def _check(self, err: int) -> int:
if err < 0:
@@ -468,7 +473,9 @@ class Transport(Wrapper):
:return: SASL object associated with this transport.
"""
- return SASL(self)
+ if not self._sasl:
+ self._sasl = SASL(self)
+ return self._sasl
def ssl(self, domain: Optional['SSLDomain'] = None, session_details:
Optional['SSLSessionDetails'] = None) -> 'SSL':
"""
@@ -512,7 +519,7 @@ class SASLException(TransportException):
pass
-class SASL(Wrapper):
+class SASL:
"""
The SASL layer is responsible for establishing an authenticated
and/or encrypted tunnel over which AMQP frames are passed between
@@ -542,9 +549,14 @@ class SASL(Wrapper):
"""
return pn_sasl_extended()
- def __init__(self, transport: Transport) -> None:
- Wrapper.__init__(self, transport._impl, pn_transport_attachments)
- self._sasl = pn_sasl(transport._impl)
+ def __new__(cls, transport: Transport) -> 'SASL':
+ if not transport._sasl:
+ sasl = super().__new__(cls)
+ sasl._sasl = pn_sasl(transport._impl)
+ transport._sasl = sasl
+ return sasl
+ else:
+ return transport._sasl
def _check(self, err):
if err < 0:
diff --git a/python/proton/_wrapper.py b/python/proton/_wrapper.py
index 88ab25990..673c2cf54 100644
--- a/python/proton/_wrapper.py
+++ b/python/proton/_wrapper.py
@@ -17,83 +17,72 @@
# under the License.
#
-from typing import Any, Callable, Optional
+from typing import Any, Callable, ClassVar, Optional
-from cproton import addressof, pn_incref, pn_decref, \
- pn_record_get_py, pn_record_def_py, pn_record_set_py
+from cproton import addressof, isnull, pn_incref, pn_decref, \
+ pn_record_get_py
from ._exceptions import ProtonException
-class EmptyAttrs:
-
- def __contains__(self, name):
- return False
-
- def __getitem__(self, name):
- raise KeyError(name)
-
- def __setitem__(self, name, value):
- raise TypeError("does not support item assignment")
-
-
-EMPTY_ATTRS = EmptyAttrs()
-
-
-class Wrapper(object):
+class Wrapper:
""" Wrapper for python objects that need to be stored in event contexts
and be retrieved again from them
Quick note on how this works:
- The actual *python* object has only 3 attributes which redirect into
the wrapped C objects:
+ The actual *python* object has only 2 attributes which redirect into
the wrapped C objects:
_impl The wrapped C object itself
_attrs This is a special pn_record_t holding a PYCTX which is a
python dict
every attribute in the python object is actually looked up here
- Because the objects actual attributes are stored away they must be
initialised *after* the wrapping
- is set up. This is the purpose of the _init method in the wrapped
object. Wrapper.__init__ will call
- eht subclass _init to initialise attributes. So they *must not* be
initialised in the subclass __init__
- before calling the superclass (Wrapper) __init__ or they will not be
accessible from the wrapper at all.
+ Because the objects actual attributes are stored away they must be
initialised *after* the wrapping. This is
+ achieved by using the __new__ method of Wrapper to create the object
with the actiual attributes before the
+ __init__ method is called.
+ In the case where an existing object is being wrapped, the __init__
method is called with the existing object
+ but should not initialise the object. This is because the object is
already initialised and the attributes
+ are already set. Use the Uninitialised method to check if the object
is already initialised.
"""
- def __init__(
- self,
- impl: Any = None,
- get_context: Optional[Callable[[Any], Any]] = None,
- constructor: Optional[Callable[[], Any]] = None
- ) -> None:
- init = False
- if impl is None and constructor is not None:
- # we are constructing a new object
- impl = constructor()
- if impl is None:
- self.__dict__["_impl"] = impl
- self.__dict__["_attrs"] = EMPTY_ATTRS
- raise ProtonException(
- "Wrapper failed to create wrapped object. Check for file
descriptor or memory exhaustion.")
- init = True
+ constructor: ClassVar[Optional[Callable[[], Any]]] = None
+ get_context: ClassVar[Optional[Callable[[Any], dict[str, Any]]]] = None
+
+ __slots__ = ["_impl", "_attrs"]
+
+ @classmethod
+ def wrap(cls, impl: Any) -> Optional['Wrapper']:
+ if isnull(impl):
+ return None
else:
- # we are wrapping an existing object
- pn_incref(impl)
+ return cls(impl)
- if get_context:
- record = get_context(impl)
+ def __new__(cls, impl: Any = None) -> 'Wrapper':
+ attrs = None
+ try:
+ if impl is None:
+ # we are constructing a new object
+ assert cls.constructor
+ impl = cls.constructor()
+ if impl is None:
+ raise ProtonException(
+ "Wrapper failed to create wrapped object. Check for
file descriptor or memory exhaustion.")
+ else:
+ # we are wrapping an existing object
+ pn_incref(impl)
+
+ assert cls.get_context
+ record = cls.get_context(impl)
attrs = pn_record_get_py(record)
- if attrs is None:
- attrs = {}
- pn_record_def_py(record)
- pn_record_set_py(record, attrs)
- init = True
- else:
- attrs = EMPTY_ATTRS
- init = False
- self.__dict__["_impl"] = impl
- self.__dict__["_attrs"] = attrs
- if init:
- self._init()
+ finally:
+ self = super().__new__(cls)
+ self._impl = impl
+ self._attrs = attrs
+ return self
+
+ def Uninitialized(self) -> bool:
+ return self._attrs == {}
def __getattr__(self, name: str) -> Any:
- attrs = self.__dict__["_attrs"]
- if name in attrs:
+ attrs = self._attrs
+ if attrs and name in attrs:
return attrs[name]
else:
raise AttributeError(name + " not in _attrs")
@@ -102,11 +91,12 @@ class Wrapper(object):
if hasattr(self.__class__, name):
object.__setattr__(self, name, value)
else:
- attrs = self.__dict__["_attrs"]
- attrs[name] = value
+ attrs = self._attrs
+ if attrs is not None:
+ attrs[name] = value
def __delattr__(self, name: str) -> None:
- attrs = self.__dict__["_attrs"]
+ attrs = self._attrs
if attrs:
del attrs[name]
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@qpid.apache.org
For additional commands, e-mail: commits-h...@qpid.apache.org
