Enable TransportType.AmqpOverWebsocket for ServiceBus - #10012
Merged
Conversation
dennispg
requested review from
KieranBrantnerMagee,
YijunXieMS,
annatisch and
yunhaoling
as code owners
February 26, 2020 23:05
|
Can one of the admins verify this patch? |
Contributor
Author
|
This seems to address #4250 for me |
dennispg
force-pushed
the
AmqpOverWebsocket
branch
from
March 5, 2020 23:42
73056d1 to
ffb068c
Compare
|
|
lmazuel
added
Client
KieranBrantnerMagee
suggested changes
Mar 13, 2020
yunhaoling
reviewed
Mar 13, 2020
yunhaoling
reviewed
Mar 13, 2020
yunhaoling
reviewed
Mar 25, 2020
yunhaoling
reviewed
Mar 25, 2020
yunhaoling
reviewed
Mar 25, 2020
yunhaoling
reviewed
Mar 25, 2020
yunhaoling
reviewed
Mar 25, 2020
yunhaoling
suggested changes
Mar 25, 2020
yunhaoling
left a comment
yunhaoling
left a comment
Contributor
There was a problem hiding this comment.
overall looks good to me, the default transport type should not be "None", it should be "TransportType.Amqp". Although it's guarded by the underlying uamqp library for now, but keeping code consistent with the doc and being explicit helps improving the readability.
Fix bug wherein async client didn't thread parsed transport type into constructor Fix bug where exception in get_running_loop is not being passed through the proper logger.
KieranBrantnerMagee
approved these changes
Mar 27, 2020
KieranBrantnerMagee
left a comment
KieranBrantnerMagee
left a comment
Member
There was a problem hiding this comment.
LGTM, thanks for the time, effort, and patience to help us by making these changes.
Don't hesitate to let me know if you had any questions about the tests I added, or the corresponding tweak in parameter plumbing in the async client.
yunhaoling
approved these changes
Mar 27, 2020
iscai-msft
added a commit
to iscai-msft/azure-sdk-for-python
that referenced
this pull request
Mar 31, 2020
…into shared_credential_search * 'master' of https://github.com/Azure/azure-sdk-for-python: [text analytics] Use AzureKeyCredential from core instead of TextAnalyticsKeyCredential (Azure#10528) Enable TransportType.AmqpOverWebsocket for ServiceBus (Azure#10012) Install tools packages in dev requirements from regression test branch (Azure#10575) Sync eng/common directory with azure-sdk-tools repository (Azure#10570)
iscai-msft
added a commit
to iscai-msft/azure-sdk-for-python
that referenced
this pull request
Mar 31, 2020
…into regenerate_v3_ta * 'master' of https://github.com/Azure/azure-sdk-for-python: [text analytics] Use AzureKeyCredential from core instead of TextAnalyticsKeyCredential (Azure#10528) Enable TransportType.AmqpOverWebsocket for ServiceBus (Azure#10012) Install tools packages in dev requirements from regression test branch (Azure#10575) Sync eng/common directory with azure-sdk-tools repository (Azure#10570) Shared credential (Azure#10509) Sync eng/common directory with azure-sdk-tools repository (Azure#10557) add back preparer (Azure#10558) initial release (Azure#10545) Release azure mgmt storage (Azure#10525) [Event Hubs] Update sample README to use absolute path (Azure#10556) Sync eng/common directory with azure-sdk-tools repository (Azure#10550)
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Just set TransportType=AmqpOverWebsocket in your connection string