Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ServiceBus dict-representation acceptance and kwarg-update functionality #14807

Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
3f05de7
Merge pull request #1 from Azure/master
bradleydamato Oct 12, 2020
488ac14
Merge remote-tracking branch 'upstream/master'
bradleydamato Oct 12, 2020
0cc38d2
Merge branch 'master' of github.com:Azure/azure-sdk-for-python
bradleydamato Oct 18, 2020
4bde81c
Merge branch 'master' of github.com:Azure/azure-sdk-for-python
bradleydamato Oct 20, 2020
fe6fcdc
Adding kwarg update line to update_* functions
bradleydamato Oct 20, 2020
b65f309
ServiceBusAdministrationClient update_* methods now accept dictionary…
bradleydamato Oct 26, 2020
30d6948
ServiceBusSender methods now accept dict-representation
bradleydamato Oct 26, 2020
d6686f1
ServiceBusSender methods now accept list of dict-representations in s…
bradleydamato Oct 27, 2020
ea060e2
Message, Async ServiceBusSender, and Async ServiceBusAdministrationCl…
bradleydamato Oct 27, 2020
0f9fc56
Reverting change made to Message._from_list() method
bradleydamato Oct 27, 2020
34e41eb
Fixing indent size
bradleydamato Oct 27, 2020
ff63098
fixing Pylint errors
bradleydamato Oct 27, 2020
260b940
Fixing update_* kwarg update logic
bradleydamato Oct 28, 2020
881d13d
added util generic functions to take in dicts and create objects + ad…
swathipil Feb 18, 2021
9462f22
updated error messages for mgmt client
swathipil Feb 18, 2021
e394824
remove unnecessary kwarg check in update_topic
swathipil Feb 18, 2021
ac17178
remove error messages
swathipil Feb 18, 2021
a81445d
changed function name
swathipil Feb 18, 2021
cc0cdd4
Merge branch 'master' into ServiceBus_dict-representation_and_kwarg_u…
swathipil Feb 18, 2021
073670e
fix merge conflict, exceptions
swathipil Feb 18, 2021
2c1fe1e
fix test errors from merge conflict
swathipil Feb 19, 2021
0f7b1b4
Merge branch 'master' into ServiceBus_dict-representation_and_kwarg_u…
swathipil Feb 19, 2021
4d20f93
added test recordings
swathipil Feb 19, 2021
3f60783
fix mypy/pylint
swathipil Feb 19, 2021
c8e20a3
adams comments + update recordings
swathipil Feb 22, 2021
97d52b4
fix mypy/pylint
swathipil Feb 22, 2021
5b787d4
fix mypy error in message
swathipil Feb 22, 2021
eef0308
remove duplicate test
swathipil Feb 22, 2021
c8bfd20
adams comments
swathipil Feb 23, 2021
315b093
anna's comments
swathipil Mar 1, 2021
5e61e23
updated _from_list to check for Mapping, not dict
swathipil Mar 1, 2021
a23aecd
update changelog
swathipil Mar 2, 2021
49e25bb
Merge branch 'master' into ServiceBus_dict-representation_and_kwarg_u…
swathipil Mar 2, 2021
ee67888
change Mapping back to dict check
swathipil Mar 2, 2021
27f40bd
Update sdk/servicebus/azure-servicebus/CHANGELOG.md
swathipil Mar 2, 2021
a38c54f
bump minor version
swathipil Mar 2, 2021
f145073
Merge branch 'ServiceBus_dict-representation_and_kwarg_update' of htt…
swathipil Mar 2, 2021
803b142
_version update
swathipil Mar 3, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions sdk/servicebus/azure-servicebus/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@

## 7.0.2 (Unreleased)

**New Features**

* Updated the following so that they accept lists and single instances of dict representations for corresponding strongly-typed object arguments:
- `update_queue`, `update_topic`, `update_subscription`, and `update_rule` on `ServiceBusAdministrationClient` accept dict representations of `QueueProperties`, `TopicProperties`, `SubscriptionProperties`, and `RuleProperties`, respectively.
- `send_messages` and `schedule_messages` on both sync and async versions of `ServiceBusSender` accept a list of or single instance of dict representations of `ServiceBusMessage`.
- `add_message` on `ServiceBusMessageBatch` now accepts a dict representation of `ServiceBusMessage`.
- Note: This is ongoing work and is the first step in supporting the above as respresentation of type `typing.Mapping`.
- Note: Thanks to bradleydamato for their large contribution to this.
swathipil marked this conversation as resolved.
Show resolved Hide resolved

**BugFixes**

* Operations failing due to `uamqp.errors.LinkForceDetach` caused by no activity on the connection for 10 minutes will now be retried internally except for the session receiver case.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@
MESSAGE_PROPERTY_MAX_LENGTH,
)

from ..exceptions import MessageSizeExceededError
from .utils import (
utc_from_timestamp,
utc_now,
transform_messages_to_sendable_if_needed,
trace_message,
create_messages_from_dicts_if_needed
)
from ..exceptions import MessageSizeExceededError

if TYPE_CHECKING:
from ..aio._servicebus_receiver_async import (
Expand Down Expand Up @@ -537,7 +538,7 @@ def __len__(self):
def _from_list(self, messages, parent_span=None):
# type: (Iterable[ServiceBusMessage], AbstractSpan) -> None
swathipil marked this conversation as resolved.
Show resolved Hide resolved
for each in messages:
if not isinstance(each, ServiceBusMessage):
if not isinstance(each, (ServiceBusMessage, dict)):
raise TypeError(
"Only ServiceBusMessage or an iterable object containing ServiceBusMessage "
"objects are accepted. Received instead: {}".format(
Expand Down Expand Up @@ -577,11 +578,14 @@ def add_message(self, message):
:rtype: None
:raises: :class: ~azure.servicebus.exceptions.MessageSizeExceededError, when exceeding the size limit.
"""

yunhaoling marked this conversation as resolved.
Show resolved Hide resolved
return self._add(message)

def _add(self, message, parent_span=None):
# type: (ServiceBusMessage, AbstractSpan) -> None
"""Actual add implementation. The shim exists to hide the internal parameters such as parent_span."""

message = create_messages_from_dicts_if_needed(message, ServiceBusMessage) # type: ignore
message = transform_messages_to_sendable_if_needed(message)
trace_message(
message, parent_span
Expand Down
44 changes: 42 additions & 2 deletions sdk/servicebus/azure-servicebus/azure/servicebus/_common/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,18 @@
import logging
import functools
import platform
from typing import Optional, Dict, Tuple, Iterable, Type, TYPE_CHECKING, Union, Iterator
from typing import (
Any,
Dict,
Iterable,
Iterator,
List,
Mapping,
Optional,
Type,
TYPE_CHECKING,
Union
)
from contextlib import contextmanager
from msrest.serialization import UTC

Expand Down Expand Up @@ -43,11 +54,26 @@
)

if TYPE_CHECKING:
from .message import ServiceBusReceivedMessage, ServiceBusMessage
from .message import ServiceBusReceivedMessage, ServiceBusMessage, ServiceBusMessageBatch
from azure.core.tracing import AbstractSpan
from .receiver_mixins import ReceiverMixin
from .._servicebus_session import BaseSession

# pylint: disable=unused-import, ungrouped-imports
DictMessageType = Union[
Mapping,
ServiceBusMessage,
List[Mapping[str, Any]],
List[ServiceBusMessage],
ServiceBusMessageBatch
]

DictMessageReturnType = Union[
ServiceBusMessage,
List[ServiceBusMessage],
ServiceBusMessageBatch
]

_log = logging.getLogger(__name__)


Expand Down Expand Up @@ -196,6 +222,20 @@ def transform_messages_to_sendable_if_needed(messages):
except AttributeError:
return messages

def create_messages_from_dicts_if_needed(messages, message_type):
# type: (DictMessageType, type) -> DictMessageReturnType
"""
This method is used to convert dict representations
of messages to a list of ServiceBusMessage objects or ServiceBusBatchMessage.
:param DictMessageType messages: A list or single instance of messages of type ServiceBusMessages or
dict representations of type ServiceBusMessage. Also accepts ServiceBusBatchMessage.
:rtype: DictMessageReturnType
"""
if isinstance(messages, list):
return [(message_type(**message) if isinstance(message, dict) else message) for message in messages]

return_messages = message_type(**messages) if isinstance(messages, dict) else messages
return return_messages

def strip_protocol_from_uri(uri):
# type: (str) -> str
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from ._common.utils import (
create_authentication,
transform_messages_to_sendable_if_needed,
create_messages_from_dicts_if_needed,
send_trace_context_manager,
trace_message,
add_link_to_send,
Expand Down Expand Up @@ -269,7 +270,9 @@ def schedule_messages(self, messages, schedule_time_utc, **kwargs):
:caption: Schedule a message to be sent in future
"""
# pylint: disable=protected-access

self._check_live()
messages = create_messages_from_dicts_if_needed(messages, ServiceBusMessage) # type: ignore
timeout = kwargs.pop("timeout", None)
if timeout is not None and timeout <= 0:
raise ValueError("The timeout must be greater than 0.")
Expand Down Expand Up @@ -365,7 +368,9 @@ def send_messages(self, message, **kwargs):
:caption: Send message.

"""

self._check_live()
message = create_messages_from_dicts_if_needed(message, ServiceBusMessage)
timeout = kwargs.pop("timeout", None)
if timeout is not None and timeout <= 0:
raise ValueError("The timeout must be greater than 0.")
Expand Down Expand Up @@ -393,11 +398,9 @@ def send_messages(self, message, **kwargs):
isinstance(message, ServiceBusMessageBatch) and len(message) == 0
): # pylint: disable=len-as-condition
return # Short circuit noop if an empty list or batch is provided.
if not isinstance(message, ServiceBusMessageBatch) and not isinstance(
message, ServiceBusMessage
):
if not isinstance(message, (ServiceBusMessageBatch, ServiceBusMessage)):
raise TypeError(
"Can only send azure.servicebus.<ServiceBusMessageBatch,ServiceBusMessage> "
"Can only send azure.servicebus.<ServiceBusMessageBatch, ServiceBusMessage> "
"or lists of ServiceBusMessage."
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from .._common import mgmt_handlers
from .._common.utils import (
transform_messages_to_sendable_if_needed,
create_messages_from_dicts_if_needed,
send_trace_context_manager,
trace_message,
add_link_to_send,
Expand Down Expand Up @@ -206,7 +207,9 @@ async def schedule_messages(
:caption: Schedule a message to be sent in future
"""
# pylint: disable=protected-access

self._check_live()
messages = create_messages_from_dicts_if_needed(messages, ServiceBusMessage) # type: ignore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did typing need to be ignored here, but not line 315?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It results in the following mypy error:
Incompatible types in assignment (expression has type "Union[ServiceBusMessage, List[ServiceBusMessage], ServiceBusMessageBatch]", variable has type "Union[ServiceBusMessage, List[ServiceBusMessage]]").

Since schedule_messages doesn't accept SBMessageBatch, reassigning messages to the result of create_messages_from_dicts_if_needed (which accepts all 3) leads to this error. send_messages doesn't do this because it accepts all 3, so reassigning message doesn't lead to incompatible types.

timeout = kwargs.pop("timeout", None)
if timeout is not None and timeout <= 0:
raise ValueError("The timeout must be greater than 0.")
Expand Down Expand Up @@ -307,7 +310,9 @@ async def send_messages(
:caption: Send message.

"""

self._check_live()
message = create_messages_from_dicts_if_needed(message, ServiceBusMessage)
timeout = kwargs.pop("timeout", None)
if timeout is not None and timeout <= 0:
raise ValueError("The timeout must be greater than 0.")
Expand All @@ -333,9 +338,7 @@ async def send_messages(
isinstance(message, ServiceBusMessageBatch) and len(message) == 0
): # pylint: disable=len-as-condition
return # Short circuit noop if an empty list or batch is provided.
if not isinstance(message, ServiceBusMessageBatch) and not isinstance(
message, ServiceBusMessage
):
if not isinstance(message, (ServiceBusMessageBatch, ServiceBusMessage)):
raise TypeError(
"Can only send azure.servicebus.<ServiceBusMessageBatch,ServiceBusMessage> "
"or lists of ServiceBusMessage."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
from ...management._utils import (
deserialize_rule_key_values,
serialize_rule_key_values,
create_properties_from_dict_if_needed,
_validate_entity_name_type,
_validate_topic_and_subscription_types,
_validate_topic_subscription_and_rule_types,
Expand Down Expand Up @@ -411,7 +412,9 @@ async def update_queue(self, queue: QueueProperties, **kwargs) -> None:
:rtype: None
"""

queue = create_properties_from_dict_if_needed(queue, QueueProperties) # type: ignore
to_update = queue._to_internal_entity()

to_update.default_message_time_to_live = avoid_timedelta_overflow(
to_update.default_message_time_to_live
)
Expand Down Expand Up @@ -636,6 +639,7 @@ async def update_topic(self, topic: TopicProperties, **kwargs) -> None:
:rtype: None
"""

topic = create_properties_from_dict_if_needed(topic, TopicProperties) # type: ignore
to_update = topic._to_internal_entity()

to_update.default_message_time_to_live = avoid_timedelta_overflow(
Expand Down Expand Up @@ -880,8 +884,10 @@ async def update_subscription(
from `get_subscription`, `update_subscription` or `list_subscription` and has the updated properties.
:rtype: None
"""

_validate_entity_name_type(topic_name, display_name="topic_name")

subscription = create_properties_from_dict_if_needed(subscription, SubscriptionProperties) # type: ignore
to_update = subscription._to_internal_entity()

to_update.default_message_time_to_live = avoid_timedelta_overflow(
Expand Down Expand Up @@ -1079,6 +1085,7 @@ async def update_rule(
"""
_validate_topic_and_subscription_types(topic_name, subscription_name)

rule = create_properties_from_dict_if_needed(rule, RuleProperties) # type: ignore
to_update = rule._to_internal_entity()

create_entity_body = CreateRuleBody(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import urllib.parse as urlparse

from azure.servicebus.management import _constants as constants
from ...management import _constants as constants
from ...management._handle_response_error import _handle_response_error

# This module defines functions get_next_template and extract_data_template.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
deserialize_rule_key_values,
serialize_rule_key_values,
extract_rule_data_template,
create_properties_from_dict_if_needed,
_validate_entity_name_type,
_validate_topic_and_subscription_types,
_validate_topic_subscription_and_rule_types,
Expand Down Expand Up @@ -404,6 +405,7 @@ def update_queue(self, queue, **kwargs):
:rtype: None
"""

queue = create_properties_from_dict_if_needed(queue, QueueProperties) # type: ignore
to_update = queue._to_internal_entity()

to_update.default_message_time_to_live = avoid_timedelta_overflow(
Expand Down Expand Up @@ -632,6 +634,7 @@ def update_topic(self, topic, **kwargs):
:rtype: None
"""

topic = create_properties_from_dict_if_needed(topic, TopicProperties) # type: ignore
to_update = topic._to_internal_entity()

to_update.default_message_time_to_live = (
Expand Down Expand Up @@ -884,8 +887,10 @@ def update_subscription(self, topic_name, subscription, **kwargs):
from `get_subscription`, `update_subscription` or `list_subscription` and has the updated properties.
:rtype: None
"""

_validate_entity_name_type(topic_name, display_name="topic_name")

subscription = create_properties_from_dict_if_needed(subscription, SubscriptionProperties) # type: ignore
to_update = subscription._to_internal_entity()

to_update.default_message_time_to_live = avoid_timedelta_overflow(
Expand Down Expand Up @@ -1077,6 +1082,7 @@ def update_rule(self, topic_name, subscription_name, rule, **kwargs):
"""
_validate_topic_and_subscription_types(topic_name, subscription_name)

rule = create_properties_from_dict_if_needed(rule, RuleProperties) # type: ignore
to_update = rule._to_internal_entity()

create_entity_body = CreateRuleBody(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,39 @@
# Licensed under the MIT License. See License.txt in the project root for license information.
# --------------------------------------------------------------------------------------------
from datetime import datetime, timedelta
from typing import cast
from typing import TYPE_CHECKING, cast, Union, Mapping
from xml.etree.ElementTree import ElementTree, SubElement, QName
import isodate
import six

from . import _constants as constants
from ._handle_response_error import _handle_response_error
if TYPE_CHECKING:
# pylint: disable=unused-import, ungrouped-imports
from ._models import QueueProperties, TopicProperties, \
SubscriptionProperties, RuleProperties, InternalQueueDescription, InternalTopicDescription, \
InternalSubscriptionDescription, InternalRuleDescription
DictPropertiesType = Union[
QueueProperties,
TopicProperties,
SubscriptionProperties,
RuleProperties,
Mapping
]
DictPropertiesReturnType = Union[
QueueProperties,
TopicProperties,
SubscriptionProperties,
RuleProperties
]

# Refer to the async version of this module under ..\aio\management\_utils.py for detailed explanation.

try:
import urllib.parse as urlparse
except ImportError:
import urlparse # type: ignore # for python 2.7

from azure.servicebus.management import _constants as constants
from ._handle_response_error import _handle_response_error


def extract_rule_data_template(feed_class, convert, feed_element):
"""Special version of function extrat_data_template for Rule.

Expand Down Expand Up @@ -307,3 +324,16 @@ def _validate_topic_subscription_and_rule_types(
type(topic_name), type(subscription_name), type(rule_name)
)
)

def create_properties_from_dict_if_needed(properties, sb_resource_type):
# type: (DictPropertiesType, type) -> DictPropertiesReturnType
"""
This method is used to create a properties object given the
resource properties type and its corresponding dict representation.
:param properties: A properties object or its dict representation.
:type properties: DictPropertiesType
:param type sb_resource_type: The type of properties object.
:rtype: DictPropertiesReturnType
"""
return_properties = sb_resource_type(**properties) if isinstance(properties, dict) else properties
return return_properties
Loading