[service-bus] AdminClient: ForwardTo wasn't applying on update due to bad property ordering in XML #14692
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ear to take effect but don't actually alter the entity. Fixes Azure#14539
richardpark-msft
requested review from
chradek,
HarshaNalluru and
ramya-rao-a
as code owners
ghost
added
the
|
/azp run js - service-bus - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ramya-rao-a
reviewed
Apr 2, 2021
sdk/servicebus/service-bus/src/serializers/queueResourceSerializer.ts
Outdated
Show resolved
Hide resolved
ramya-rao-a
reviewed
Apr 2, 2021
sdk/servicebus/service-bus/src/serializers/subscriptionResourceSerializer.ts
Outdated
Show resolved
Hide resolved
ramya-rao-a
reviewed
Apr 2, 2021
sdk/servicebus/service-bus/src/serializers/queueResourceSerializer.ts
Outdated
Show resolved
Hide resolved
added 2 commits
April 2, 2021 16:14
|
BTW, the analyze step is failing due to unformatted files |
chradek
reviewed
Apr 2, 2021
sdk/servicebus/service-bus/src/serializers/queueResourceSerializer.ts
Outdated
Show resolved
Hide resolved
added 5 commits
April 2, 2021 18:52
|
/azp run js - service-bus - tests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Closed
6 tasks
chradek
approved these changes
Apr 5, 2021
sdk/servicebus/service-bus/src/serializers/queueResourceSerializer.ts
Outdated
Show resolved
Hide resolved
| DefaultMessageTimeToLive: topic.defaultMessageTimeToLive, | ||
| MaxSizeInMegabytes: getStringOrUndefined(topic.maxSizeInMegabytes), | ||
| RequiresDuplicateDetection: getStringOrUndefined(topic.requiresDuplicateDetection), | ||
| DuplicateDetectionHistoryTimeWindow: topic.duplicateDetectionHistoryTimeWindow, | ||
| EnableBatchedOperations: getStringOrUndefined(topic.enableBatchedOperations), | ||
| // TODO: in .net, but not in here: FilteringMessagesBeforePublishing |
There was a problem hiding this comment.
Are these TODOs meant to track adding these fields? Can we create an issue for them?
There was a problem hiding this comment.
I'll log an issue for us for this and the one below and remove the comment.
ramya-rao-a
reviewed
Apr 5, 2021
sdk/servicebus/service-bus/src/serializers/topicResourceSerializer.ts
Outdated
Show resolved
Hide resolved
ramya-rao-a
reviewed
Apr 5, 2021
sdk/servicebus/service-bus/src/serializers/queueResourceSerializer.ts
Outdated
Show resolved
Hide resolved
ramya-rao-a
approved these changes
Apr 5, 2021
| DefaultMessageTimeToLive: topic.defaultMessageTimeToLive, | ||
| MaxSizeInMegabytes: getStringOrUndefined(topic.maxSizeInMegabytes), | ||
| RequiresDuplicateDetection: getStringOrUndefined(topic.requiresDuplicateDetection), | ||
| DuplicateDetectionHistoryTimeWindow: topic.duplicateDetectionHistoryTimeWindow, | ||
| EnableBatchedOperations: getStringOrUndefined(topic.enableBatchedOperations), | ||
| // TODO: in .net, but not in here: FilteringMessagesBeforePublishing |
There was a problem hiding this comment.
Suggested change
| // TODO: in .net, but not in here: FilteringMessagesBeforePublishing |
sdk/servicebus/service-bus/src/serializers/topicResourceSerializer.ts
Outdated
Show resolved
Hide resolved
|
Hello @HarshaNalluru! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
ghost
merged commit
HarshaNalluru
pushed a commit
to HarshaNalluru/azure-sdk-for-js
that referenced
this pull request
Apr 5, 2021
… bad property ordering in XML (Azure#14692) The ordering of properties in the XML requests to the Service Bus ATOM API is significant and changing it can have side effects. In this instance, the ordering issues caused us to appear to have setup forwarding properly for a queue but forwarding was NOT actually enabled. Our previous testing missed this because this data actually round-trips properly through the API but it doesn't trigger whatever actual setting needs to happen to cause forwarding to happen. This PR reconciles our queue, topic and subscription entities ordering against the .net layer, which acts as the de-facto authority. Fixes Azure#14539
ghost
pushed a commit
that referenced
this pull request
Apr 5, 2021
AdminClient: ForwardTo wasn't applying on update due to bad property ordering in XML (#14692) The ordering of properties in the XML requests to the Service Bus ATOM API is significant and changing it can have side effects. In this instance, the ordering issues caused us to appear to have setup forwarding properly for a queue but forwarding was NOT actually enabled. Our previous testing missed this because this data actually round-trips properly through the API but it doesn't trigger whatever actual setting needs to happen to cause forwarding to happen. This PR reconciles our queue, topic and subscription entities ordering against the .net layer, which acts as the de-facto authority. Fixes #14539
jay-most
pushed a commit
to jay-most/azure-sdk-for-js
that referenced
this pull request
Apr 26, 2021
… bad property ordering in XML (Azure#14692) The ordering of properties in the XML requests to the Service Bus ATOM API is significant and changing it can have side effects. In this instance, the ordering issues caused us to appear to have setup forwarding properly for a queue but forwarding was NOT actually enabled. Our previous testing missed this because this data actually round-trips properly through the API but it doesn't trigger whatever actual setting needs to happen to cause forwarding to happen. This PR reconciles our queue, topic and subscription entities ordering against the .net layer, which acts as the de-facto authority. Fixes Azure#14539
openapi-sdkautomation bot
pushed a commit
to AzureSDKAutomation/azure-sdk-for-js
that referenced
this pull request
Jul 15, 2021
Adding compute 2021-04-01 version (Azure#15100) * initial copy of old files to new folder 2021-04-01 * api version update * Add SpotRestorePolicy to CRP's VM Scale Set Resource (Azure#14692) * Resolve merge conflicts after branch recreation * Fix Lint error * update with changes mae in 2021-03-01 * Update compute.json * Update CreateAVmWithNetworkInterfaceConfiguration.json update example file also to fix breaking change naming issue. * Feature/cplat 2021 04 01 - Capacity Reservation related changes (Azure#15058) * updated code * updated swagger with cr changes * updated swagger with cr changes * updated swagger with cr changes * updated swagger * updated swagger * updated swagger * Feature/cplat 2021 04 01 (Azure#15070) * 2021-04 initial * 2021-04 examples and readme * adding CR VMSS changes * updating api version in example and description of CR profile Co-authored-by: Adam Sandor <[email protected]> Co-authored-by: Theodore Chang <[email protected]> * Update compute.json * syncronizing 2021-04-01 swagger with changes made in 2021-03-01 Co-authored-by: Sandeep Vishnu <[email protected]> Co-authored-by: hari-bodicherla <[email protected]> Co-authored-by: micahjo <[email protected]> Co-authored-by: Adam Sandor <[email protected]>
This pull request was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The ordering of properties in the XML requests to the Service Bus ATOM API is significant and changing it can have side effects.
In this instance, the ordering issues caused us to appear to have setup forwarding properly for a queue but forwarding was NOT actually enabled. Our previous testing missed this because this data actually round-trips properly through the API but it doesn't trigger whatever actual setting needs to happen to cause forwarding to happen.
This PR reconciles our queue, topic and subscription entities ordering against the .net layer, which acts as the de-facto authority.
Fixes #14539