-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[Service Bus] ATOM Management API Proposal #9116
[Service Bus] ATOM Management API Proposal #9116
Conversation
deleteTopic(topicName: string): Promise<DeleteTopicResponse>; | ||
getQueue(queueName: string): Promise<GetQueueResponse>; | ||
getQueueMetrics(queueName: string): Promise<GetQueueMetricsResponse>; | ||
getQueues(listRequestOptions?: ListRequestOptions): Promise<GetQueuesResponse>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
listRequestOptions?: ListRequestOptions
to
skip?: number, top?: number
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
listRequestOptions
-> options
@@ -72,6 +208,63 @@ export interface OperationOptions { | |||
tracingOptions?: OperationTracingOptions; | |||
} | |||
|
|||
// @public | |||
export interface QueueMetrics { | |||
accessedOn?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In .NET, it is accessedAt
instead of accessedOn
. SImilarly, others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be of type Date
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it should be Date. Don't we have guidelines on which prefix to use at vs on?
cc @ShivangiReja
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HarshaNalluru You can change this in the follow up PR
…to harshan/sb/issue/atom-api
Hey @HarshaNalluru,
|
Test run long back - https://dev.azure.com/azure-sdk/internal/_build/results?buildId=408805&view=logs&j=bbd7d436-e05d-51a8-99a8-0701dc9dffeb&t=05339ee9-4cff-5853-bdb5-a48cd27b2510 I believe most of the tests are fixed now. I'll trigger the pipeline again. |
/azp run js - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run js - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, rest of the changes we can do in the follow up PR.
Please merge once you get the tests green
/azp run js - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run js - servicebus - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/check-enforcer evaluate |
Bringing back the ATOM API..
Restructuring the existing API according to the .NET proposal reviewed by Krzysztof and the service team.
.NET Proposal - https://apiview.dev/Assemblies/Review/b5d7e3304000449c859c2016f718db52#Azure.Messaging.ServiceBus.Management.ServiceBusManagementClient
There are some missing functionalities in our API when compared to .NET,
splitting this PR into two as suggested by @ramya-rao-a to add the missing APIs in the followup PR #9221.
This PR mainly focuses on restructuring the API and I've tried to avoid adding new APIs and any major changes to the tests.
Contents of the upcoming PR #9221
(Implementation and testing ongoing)