Skip to content

[Test Proxy] Support AsyncioRequestsTransport - #25897

Merged
laiapat merged 1 commit into
Azure:mainfrom
laiapat:proxy-asyncio-transport
Sep 10, 2022
Merged

[Test Proxy] Support AsyncioRequestsTransport#25897
laiapat merged 1 commit into
Azure:mainfrom
laiapat:proxy-asyncio-transport

Conversation

@laiapat

@laiapat laiapat commented Aug 26, 2022

Copy link
Copy Markdown
Member

Description

We currently only patch AioHttpTransport, the most common async transport in the SDK. App Configuration uses AsyncioRequestsTransport as its async transport though -- whether or not it should hard code this is being discussed separately, but in any case our test proxy infrastructure should support any transport that clients choose to use.

This adds support for AsyncioRequestsTransport by patching its send method during recorded tests, just as we currently do for AioHttpTransport.

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.
    • Tested using App Configuration tests.

@laiapat laiapat added EngSys This issue is impacting the engineering system. test-enhancement labels Aug 26, 2022
@laiapat
laiapat requested a review from scbedd August 26, 2022 23:15
@laiapat

laiapat commented Aug 29, 2022

Copy link
Copy Markdown
Member Author

Just talked to Laurent about this. His opinion is that the test proxy should be allowed to depend on a single async transport -- in this case, AioHttpTransport. The problem is that App Config currently has some tests fail if AioHttpTransport is used with clients.

The solution should be to fix App Config's dependence on AsyncioRequestsTransport so that tests can use azure-core's default async transport (AioHttpTransport). That's going to take some investigation since there are some tests failing with AioHttpTransport today, so I'll leave this PR up in case we need test proxy support to get App Config migrated. (Or if we change our minds and decide to support more than just AioHttpTransport)

@laiapat

laiapat commented Sep 10, 2022

Copy link
Copy Markdown
Member Author

Merging, per #26140 (comment). We can drop this support later on once App Config has proper support for AioHttpTransport, but for now the lack of AsyncioRequestsTransport in the test proxy is blocking their migration

@laiapat
laiapat merged commit e997e0c into Azure:main Sep 10, 2022
@laiapat
laiapat deleted the proxy-asyncio-transport branch September 10, 2022 01:02
laiapat added a commit to laiapat/azure-sdk-for-python that referenced this pull request Sep 22, 2022
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-python that referenced this pull request Oct 25, 2023
Add 2023-11-01-preview for appplatform (Azure#26231)

* Adds base for updating Microsoft.AppPlatform from version preview/2023-09-01-preview to version 2023-11-01-preview

* Updates readme

* Updates API version in new specs and examples

* Fix go readme for asa api (Azure#25897)

* Update language specific readme.md for 2023-11-01-preview api version (Azure#25898)

* add more build result information in build resource (Azure#25930)

Co-authored-by: 刘清仪 <liuqingyi@liuqingyideMacBook-Pro.local>

* Add apiTryOutEnabledState for api protal (Azure#25923)

* Add swagger for Spring Cloud Gateway response cache (Azure#25924)

* Update appplatform.json (Azure#26234)

* Update appplatform.json

* Update appplatform.json

---------

Co-authored-by: Sandy Cai <sandy.caiqing@gmail.com>
Co-authored-by: Qingyi Liu <34502364+smile37773@users.noreply.github.com>
Co-authored-by: 刘清仪 <liuqingyi@liuqingyideMacBook-Pro.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

EngSys This issue is impacting the engineering system. test-enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants