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.

Sign up for GitHub

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] Rename entity_availability_status to availability_status #13629

Merged
merged 3 commits into from
Sep 9, 2020
Merged

[ServiceBus] Rename entity_availability_status to availability_status #13629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d16d19
- Rename `entity_availability_status` to `availability_status`
KieranBrantnerMagee Sep 8, 2020
eff3ee2
Merge from upstream master
KieranBrantnerMagee Sep 8, 2020
5805eb9
Merge from upstream master
KieranBrantnerMagee Sep 9, 2020
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
1 change: 1 addition & 0 deletions sdk/servicebus/azure-servicebus/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
* Remove `support_ordering` from `create_queue` and `QueueProperties`
* Remove `enable_subscription_partitioning` from `create_topic` and `TopicProperties`
* `get_dead_letter_[queue,subscription]_receiver()` has been removed. To connect to a dead letter queue, utilize the `sub_queue` parameter of `get_[queue,subscription]_receiver()` provided with a value from the `SubQueue` enum
* Rename `entity_availability_status` to `availability_status`

## 7.0.0b5 (2020-08-10)

Expand Down
6 changes: 3 additions & 3 deletions sdk/servicebus/azure-servicebus/azure/servicebus/aio/management/_management_client_async.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ async def create_queue(self, name: str, **kwargs) -> QueueProperties:
dead_lettering_on_message_expiration=kwargs.pop("dead_lettering_on_message_expiration", None),
default_message_time_to_live=kwargs.pop("default_message_time_to_live", None),
duplicate_detection_history_time_window=kwargs.pop("duplicate_detection_history_time_window", None),
entity_availability_status=kwargs.pop("entity_availability_status", None),
availability_status=None,
Copy link
Contributor

@yunhaoling yunhaoling Sep 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

for confirmation: it's not a settable property (from the perspective of sdk client) on the entity? -- it's something the service side controls.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct++

Copy link
Member

@lmazuel lmazuel Sep 8, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@KieranBrantnerMagee so it means the kwargs pop was a mistake? I was ignored before? Was creating an error? What happened?

Copy link
Member Author

@KieranBrantnerMagee KieranBrantnerMagee Sep 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

"Mistake" isn't the word I'd use. It should not be settable; it's effectively a read-only setting although it's theoretically exposed on the parameters that can be passed to create. (If this is what you're getting at, it does beg the question of whether it should be addressed in swagger; I made some assumptions that I might have to reassess if you would think it would)

As a note: don't want to cut off this discussion since I know you're off for the day and we'll be checking this in for code-complete, so feel free to ping async tomorrow.

enable_batched_operations=kwargs.pop("enable_batched_operations", None),
enable_express=kwargs.pop("enable_express", None),
enable_partitioning=kwargs.pop("enable_partitioning", None),
Expand Down Expand Up @@ -439,7 +439,7 @@ async def create_topic(self, name: str, **kwargs) -> TopicProperties:
support_ordering=kwargs.pop("support_ordering", None),
auto_delete_on_idle=kwargs.pop("auto_delete_on_idle", None),
enable_partitioning=kwargs.pop("enable_partitioning", None),
entity_availability_status=kwargs.pop("entity_availability_status", None),
availability_status=None,
enable_express=kwargs.pop("enable_express", None),
user_metadata=kwargs.pop("user_metadata", None)
)
Expand Down Expand Up @@ -654,7 +654,7 @@ async def create_subscription(
user_metadata=kwargs.pop("user_metadata", None),
forward_dead_lettered_messages_to=kwargs.pop("forward_dead_lettered_messages_to", None),
auto_delete_on_idle=kwargs.pop("auto_delete_on_idle", None),
entity_availability_status=kwargs.pop("entity_availability_status", None),
availability_status=None,
)
to_create = subscription._to_internal_entity() # type: ignore # pylint:disable=protected-access

Expand Down
6 changes: 3 additions & 3 deletions sdk/servicebus/azure-servicebus/azure/servicebus/management/_management_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ def create_queue(self, name, **kwargs):
dead_lettering_on_message_expiration=kwargs.pop("dead_lettering_on_message_expiration", None),
default_message_time_to_live=kwargs.pop("default_message_time_to_live", None),
duplicate_detection_history_time_window=kwargs.pop("duplicate_detection_history_time_window", None),
entity_availability_status=kwargs.pop("entity_availability_status", None),
availability_status=None,
enable_batched_operations=kwargs.pop("enable_batched_operations", None),
enable_express=kwargs.pop("enable_express", None),
enable_partitioning=kwargs.pop("enable_partitioning", None),
Expand Down Expand Up @@ -442,7 +442,7 @@ def create_topic(self, name, **kwargs):
support_ordering=kwargs.pop("support_ordering", None),
auto_delete_on_idle=kwargs.pop("auto_delete_on_idle", None),
enable_partitioning=kwargs.pop("enable_partitioning", None),
entity_availability_status=kwargs.pop("entity_availability_status", None),
availability_status=None,
enable_express=kwargs.pop("enable_express", None),
user_metadata=kwargs.pop("user_metadata", None)
)
Expand Down Expand Up @@ -663,7 +663,7 @@ def create_subscription(self, topic, name, **kwargs):
user_metadata=kwargs.pop("user_metadata", None),
forward_dead_lettered_messages_to=kwargs.pop("forward_dead_lettered_messages_to", None),
auto_delete_on_idle=kwargs.pop("auto_delete_on_idle", None),
entity_availability_status=kwargs.pop("entity_availability_status", None),
availability_status=None,
)
to_create = subscription._to_internal_entity() # type: ignore # pylint:disable=protected-access

Expand Down
30 changes: 15 additions & 15 deletions sdk/servicebus/azure-servicebus/azure/servicebus/management/_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ class QueueProperties(DictMixin): # pylint:disable=too-many-instance-attributes
:ivar duplicate_detection_history_time_window: ISO 8601 timeSpan structure that defines the
duration of the duplicate detection history. The default value is 10 minutes.
:type duplicate_detection_history_time_window: ~datetime.timedelta
:ivar entity_availability_status: Availibility status of the entity. Possible values include:
:ivar availability_status: Availibility status of the entity. Possible values include:
"Available", "Limited", "Renaming", "Restoring", "Unknown".
:type entity_availability_status: str or
:type availability_status: str or
~azure.servicebus.management.EntityAvailabilityStatus
:ivar enable_batched_operations: Value that indicates whether server-side batched operations
are enabled.
Expand Down Expand Up @@ -172,7 +172,7 @@ def __init__(
self.dead_lettering_on_message_expiration = extract_kwarg('dead_lettering_on_message_expiration')
self.default_message_time_to_live = extract_kwarg('default_message_time_to_live')
self.duplicate_detection_history_time_window = extract_kwarg('duplicate_detection_history_time_window')
self.entity_availability_status = extract_kwarg('entity_availability_status')
self.availability_status = extract_kwarg('availability_status')
self.enable_batched_operations = extract_kwarg('enable_batched_operations')
self.enable_express = extract_kwarg('enable_express')
self.enable_partitioning = extract_kwarg('enable_partitioning')
Expand All @@ -199,7 +199,7 @@ def _from_internal_entity(cls, name, internal_qd):
dead_lettering_on_message_expiration=internal_qd.dead_lettering_on_message_expiration,
default_message_time_to_live=internal_qd.default_message_time_to_live,
duplicate_detection_history_time_window=internal_qd.duplicate_detection_history_time_window,
entity_availability_status=internal_qd.entity_availability_status,
availability_status=internal_qd.entity_availability_status,
enable_batched_operations=internal_qd.enable_batched_operations,
enable_express=internal_qd.enable_express,
enable_partitioning=internal_qd.enable_partitioning,
Expand All @@ -226,7 +226,7 @@ def _to_internal_entity(self):
self._internal_qd.dead_lettering_on_message_expiration = self.dead_lettering_on_message_expiration
self._internal_qd.default_message_time_to_live = self.default_message_time_to_live
self._internal_qd.duplicate_detection_history_time_window = self.duplicate_detection_history_time_window
self._internal_qd.entity_availability_status = self.entity_availability_status
self._internal_qd.entity_availability_status = self.availability_status
self._internal_qd.enable_batched_operations = self.enable_batched_operations
self._internal_qd.enable_express = self.enable_express
self._internal_qd.enable_partitioning = self.enable_partitioning
Expand Down Expand Up @@ -389,9 +389,9 @@ class TopicProperties(DictMixin): # pylint:disable=too-many-instance-attributes
:ivar enable_partitioning: A value that indicates whether the topic is to be partitioned
across multiple message brokers.
:type enable_partitioning: bool
:ivar entity_availability_status: Availability status of the entity. Possible values include:
:ivar availability_status: Availability status of the entity. Possible values include:
"Available", "Limited", "Renaming", "Restoring", "Unknown".
:type entity_availability_status: str or
:type availability_status: str or
~azure.servicebus.management.EntityAvailabilityStatus
:ivar enable_express: A value that indicates whether Express Entities are enabled. An express
queue holds a message in memory temporarily before writing it to persistent storage.
Expand Down Expand Up @@ -422,7 +422,7 @@ def __init__(
self.support_ordering = extract_kwarg('support_ordering')
self.auto_delete_on_idle = extract_kwarg('auto_delete_on_idle')
self.enable_partitioning = extract_kwarg('enable_partitioning')
self.entity_availability_status = extract_kwarg('entity_availability_status')
self.availability_status = extract_kwarg('availability_status')
self.enable_express = extract_kwarg('enable_express')
self.user_metadata = extract_kwarg('user_metadata')

Expand All @@ -444,7 +444,7 @@ def _from_internal_entity(cls, name, internal_td):
support_ordering=internal_td.support_ordering,
auto_delete_on_idle=internal_td.auto_delete_on_idle,
enable_partitioning=internal_td.enable_partitioning,
entity_availability_status=internal_td.entity_availability_status,
availability_status=internal_td.entity_availability_status,
enable_express=internal_td.enable_express,
user_metadata=internal_td.user_metadata
)
Expand All @@ -466,7 +466,7 @@ def _to_internal_entity(self):
self._internal_td.support_ordering = self.support_ordering
self._internal_td.auto_delete_on_idle = self.auto_delete_on_idle
self._internal_td.enable_partitioning = self.enable_partitioning
self._internal_td.entity_availability_status = self.entity_availability_status
self._internal_td.entity_availability_status = self.availability_status
self._internal_td.enable_express = self.enable_express
self._internal_td.user_metadata = self.user_metadata

Expand Down Expand Up @@ -591,9 +591,9 @@ class SubscriptionProperties(DictMixin): # pylint:disable=too-many-instance-att
:ivar auto_delete_on_idle: ISO 8601 timeSpan idle interval after which the subscription is
automatically deleted. The minimum duration is 5 minutes.
:type auto_delete_on_idle: ~datetime.timedelta
:ivar entity_availability_status: Availability status of the entity. Possible values include:
:ivar availability_status: Availability status of the entity. Possible values include:
"Available", "Limited", "Renaming", "Restoring", "Unknown".
:type entity_availability_status: str or
:type availability_status: str or
~azure.servicebus.management.EntityAvailabilityStatus
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we consider renaming azure.servicebus.management.EntityAvailabilityStatus to azure.servicebus.management.AvailabilityStatus for consistency?

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered that, and was wondering if you or others would mention as well.

My reason for leaning towards no was that in isolation AvailabilityStatus seemed imprecise, and the added term narrowed its use such that, for being located in a somewhat generic module path, it'd be more clear what it's intended for.

Let me know what you think; I'll also cc @lmazuel to get a fresh pair of eyes.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a fair point. Entity prefix makes it more clear whose status it's describing.
I see .NET Track1 is already doing such: https://docs.microsoft.com/en-us/dotnet/api/microsoft.servicebus.messaging.entityavailabilitystatus?view=azure-dotnet

just being over paranoid here: is the naming EntityAvailabilityStatus consistent among languages?

Copy link
Member Author

Choose a reason for hiding this comment

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

Now I'm paranoid because I can't find it in the apiviews for either .net or java, but I have the issue here as well where it's noted that we're keeping it with the shortened name. I've reached out to folks to try and see what the status is.

Copy link
Member Author

Choose a reason for hiding this comment

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

So it's good we synced on this, there were some deltas. Java didn't know they needed this because .net didn't have it in their apiview, as it was pending for them, so we're ahead of the pack.

Given this, I'm going to leave with my proposal for the time being and see if anyone flags it/we can have a proper design discussion where I can voice what I shared with you above. More than happy to go either way if we do it intentionally.

LMK how this sounds to you.

Copy link
Member

Choose a reason for hiding this comment

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

Personally I don't mind both, I let you decide.

"""
def __init__(self, name, **kwargs):
Expand All @@ -617,7 +617,7 @@ def __init__(self, name, **kwargs):
self.user_metadata = extract_kwarg('user_metadata')
self.forward_dead_lettered_messages_to = extract_kwarg('forward_dead_lettered_messages_to')
self.auto_delete_on_idle = extract_kwarg('auto_delete_on_idle')
self.entity_availability_status = extract_kwarg('entity_availability_status')
self.availability_status = extract_kwarg('availability_status')

validate_extraction_missing_args(extraction_missing_args)

Expand All @@ -639,7 +639,7 @@ def _from_internal_entity(cls, name, internal_subscription):
user_metadata=internal_subscription.user_metadata,
forward_dead_lettered_messages_to=internal_subscription.forward_dead_lettered_messages_to,
auto_delete_on_idle=internal_subscription.auto_delete_on_idle,
entity_availability_status=internal_subscription.entity_availability_status
availability_status=internal_subscription.entity_availability_status
)
subscription._internal_sd = deepcopy(internal_subscription)
return subscription
Expand All @@ -661,7 +661,7 @@ def _to_internal_entity(self):
self._internal_sd.user_metadata = self.user_metadata
self._internal_sd.forward_dead_lettered_messages_to = self.forward_dead_lettered_messages_to
self._internal_sd.auto_delete_on_idle = self.auto_delete_on_idle
self._internal_sd.entity_availability_status = self.entity_availability_status
self._internal_sd.entity_availability_status = self.availability_status

return self._internal_sd

Expand Down
2 changes: 1 addition & 1 deletion sdk/servicebus/azure-servicebus/tests/async_tests/mgmt_tests/test_mgmt_queues_async.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ async def test_async_mgmt_queue_create_by_name(self, servicebus_namespace_connec
try:
queue = await mgmt_service.get_queue(queue_name)
assert queue.name == queue_name
assert queue.entity_availability_status == 'Available'
assert queue.availability_status == 'Available'
assert queue.status == 'Active'
# assert created_at < queue.created_at < utc_now() + datetime.timedelta(minutes=10) # TODO: Should be created_at_utc for consistency with dataplane.
finally:
Expand Down
2 changes: 1 addition & 1 deletion ...servicebus/azure-servicebus/tests/async_tests/mgmt_tests/test_mgmt_subscriptions_async.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ async def test_async_mgmt_subscription_create_by_name(self, servicebus_namespace
await mgmt_service.create_subscription(topic_name, subscription_name)
subscription = await mgmt_service.get_subscription(topic_name, subscription_name)
assert subscription.name == subscription_name
assert subscription.entity_availability_status == 'Available'
assert subscription.availability_status == 'Available'
assert subscription.status == 'Active'
finally:
await mgmt_service.delete_subscription(topic_name, subscription_name)
Expand Down
2 changes: 1 addition & 1 deletion sdk/servicebus/azure-servicebus/tests/async_tests/mgmt_tests/test_mgmt_topics_async.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ async def test_async_mgmt_topic_create_by_name(self, servicebus_namespace_connec
await mgmt_service.create_topic(topic_name)
topic = await mgmt_service.get_topic(topic_name)
assert topic.name == topic_name
assert topic.entity_availability_status == 'Available'
assert topic.availability_status == 'Available'
assert topic.status == 'Active'
finally:
await mgmt_service.delete_topic(topic_name)
Expand Down
2 changes: 1 addition & 1 deletion sdk/servicebus/azure-servicebus/tests/mgmt_tests/test_mgmt_queues.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ def test_mgmt_queue_create_by_name(self, servicebus_namespace_connection_string,
try:
queue = mgmt_service.get_queue(queue_name)
assert queue.name == queue_name
assert queue.entity_availability_status == 'Available'
assert queue.availability_status == 'Available'
assert queue.status == 'Active'
# assert created_at < queue.created_at < utc_now() + datetime.timedelta(minutes=10) # TODO: Should be created_at_utc for consistency with dataplane.
finally:
Expand Down
2 changes: 1 addition & 1 deletion sdk/servicebus/azure-servicebus/tests/mgmt_tests/test_mgmt_subscriptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def test_mgmt_subscription_create_by_name(self, servicebus_namespace_connection_
mgmt_service.create_subscription(topic_name, subscription_name)
subscription = mgmt_service.get_subscription(topic_name, subscription_name)
assert subscription.name == subscription_name
assert subscription.entity_availability_status == 'Available'
assert subscription.availability_status == 'Available'
assert subscription.status == 'Active'
finally:
mgmt_service.delete_subscription(topic_name, subscription_name)
Expand Down
2 changes: 1 addition & 1 deletion sdk/servicebus/azure-servicebus/tests/mgmt_tests/test_mgmt_topics.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def test_mgmt_topic_create_by_name(self, servicebus_namespace_connection_string,
mgmt_service.create_topic(topic_name)
topic = mgmt_service.get_topic(topic_name)
assert topic.name == topic_name
assert topic.entity_availability_status == 'Available'
assert topic.availability_status == 'Available'
assert topic.status == 'Active'
finally:
mgmt_service.delete_topic(topic_name)
Expand Down