Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Management Operations #481

Merged
merged 106 commits into from
Jun 28, 2018
Merged

Management Operations #481

merged 106 commits into from
Jun 28, 2018

Conversation

nemakam
Copy link
Contributor

@nemakam nemakam commented Jun 7, 2018

Introduces Serivce Bus Management operations - CRUD of entities
Fixes #65

The API is exposed through ManagementClient which uses HTTP underneath to perform these operations.
Summary of exposed operations:

  1. GetQueue returns the QueueDescription which contains static information about the queue.
  2. QueueDescription is required to Create or Update the queue.
  3. ManagementClient.GetQueueRuntimeInfo returns QueueRuntimeInfo which contains runtime information of the queue like the message count and the size.
  4. GetQueues will return list of QueueDescription and will not contain runtime information.

Majority of the operations are similar to the ones exposed in older client (Microsoft.ServiceBus -> NamespaceManager), with following differences:

  1. GetQueue now returns only static information as part of QueueDescription and runtime information is part of QueueRuntimeInfo.
  2. Default value of DuplicateDetection feature is 1 minute.

@nemakam nemakam requested a review from a team as a code owner June 7, 2018 22:46
public static readonly TimeSpan MinimumDuplicateDetectionHistoryTimeWindow = TimeSpan.FromSeconds(20);
public static readonly int MinAllowedMaxDeliveryCount = 1;
public static readonly int MinAllowedMaxEntitySizeInGB = 1;
public static readonly int MaxAllowedMaxEntitySizeInGB = 5;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will not be right for the Premium tier, won't it?

private string namespaceConnectionString;

// TODO: Expose other constructor overloads
public ManagementClient(ServiceBusConnectionStringBuilder connectionStringBuilder)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Regarding ctor and TODO comment - management client shoudl by default receive connection string and internally exercise ServiceBusConnectionStringBuilder. While it's not that difficult to write

new ManagementClient(new ServiceBusConnectionStringBuilder(connectionString));

customers will appreciate just doing

new ManagementClient(connectionString);

{
// TODO: Document all exceptions that might be thrown
// TODO: Retry for transient
public class ManagementClient : ClientEntity, IManagementClient
Copy link
Collaborator

@SeanFeldman SeanFeldman Jun 8, 2018

Choose a reason for hiding this comment

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

What ClientEntity is for? If for connection, shouldn't it be incapsulated rather than ingerited? #ByDesign

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It gives you IsClosedOrClosing, Connection, Timeout, RetryPolicy and CloseAsync().


In reply to: 194202130 [](ancestors = 194202130)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are those needed for ManagementClient internally? From the API standpoint it's not obvious that these are needed (looking at IManagementClient

Copy link
Collaborator

Choose a reason for hiding this comment

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

@nemakam it still feels off to implement ClientEntity rather than encapsulate one internally. Connection string would give you what you need, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What part of that feels off?
Like I mentioned, we still need IsClosedOrClosing, and same thing with OperationTimeout, RetryPolicy and CloseAsync().

namespace Microsoft.Azure.ServiceBus.Management
{
// TODO: Input validation
public class QueueDescription : IEquatable<QueueDescription>
Copy link
Collaborator

@SeanFeldman SeanFeldman Jun 8, 2018

Choose a reason for hiding this comment

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

Looks like de-dup related properties are missing #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. Its present. Ctrl+F "Duplicate"


In reply to: 194202383 [](ancestors = 194202383)

@SeanFeldman
Copy link
Collaborator

SeanFeldman commented Jun 8, 2018

@nemakam ManagementClient should also expose namespace SKU. #WontFix

@nemakam nemakam changed the title [WIP] Management Operations Management Operations Jun 23, 2018
@SeanFeldman
Copy link
Collaborator

@nemakam is there anything else on this PR or could it be merged/released?

@nemakam
Copy link
Contributor Author

nemakam commented Jun 27, 2018

@SeanFeldman
Waiting for the internal review to finish... Will publish the preview package tomorrow...
Meanwhile, your review status is also in requested changes :)

@SeanFeldman
Copy link
Collaborator

haha, that's ancient history 😄
image

makam added 2 commits June 28, 2018 13:12
1. Moving public methods before private
2. Constants cleanup
@nemakam nemakam merged commit 58f08b7 into dev Jun 28, 2018
@nemakam nemakam deleted the management branch June 28, 2018 21:39
@nemakam nemakam added this to the 3.1.0 milestone Jun 28, 2018
@SeanFeldman
Copy link
Collaborator

🎉 🎈 🎆

@mookid8000
Copy link

How weird and sad that it should take so long to get queue and topic creation capabilities back in the driver, but it's so awesome that it's finally in! 👍 🎉 🌮

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants