Skip to content
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

[azure-core] Small fixes for aiohttp #6490

Merged
merged 12 commits into from
Jul 26, 2019
Merged

Conversation

annatisch
Copy link
Member

There are 3 fixes in this PR:

  • Aiohttp does not define an error for request read timeouts - instead it relies on the async future to timeout. The patch I have here catches the asyncio.TimeoutError and raises the ServiceResponseError - however I'm not entirely sure this correct - given that there may be other reasons the future times-out. Either way, whichever is raised here, the retry policy must be updated to work with it.
  • Aiohttp ClientResponse object has a "status" attribute rather than the "status_code" used by Requests.
  • Aiohttp uses a single string for the content-type headers rather than the list used by Requests.

@annatisch annatisch added Client This issue points to a problem in the data-plane of the library. Azure.Core labels Jul 25, 2019
@annatisch annatisch requested review from bryevdv and johanste July 25, 2019 16:53
@annatisch annatisch requested a review from xiangyan99 as a code owner July 25, 2019 16:53
@adxsdk6
Copy link

adxsdk6 commented Jul 25, 2019

Can one of the admins verify this patch?

@annatisch annatisch requested review from chlowell and iscai-msft July 25, 2019 20:21
@annatisch
Copy link
Member Author

Added another fix to this PR - found by @iscai-msft
There was an issue with a couple of the async transports inheriting the synchronous sleep function which then failed when called as a coroutine (e.g. in the async retry policy).

Sorry to add another feature @bryevdv ....

@annatisch annatisch requested a review from lmazuel July 25, 2019 23:27
@annatisch
Copy link
Member Author

@chlowell - I remember at one point we put handling into azure-core because aiohttp (iirc) was returning the content-type header as a list rather than a string. This no longer seems to be the case... can you recall the particulars? Or am I getting completely confused with something else?

@annatisch annatisch requested a review from schaabs as a code owner July 26, 2019 00:17
@lmazuel
Copy link
Member

lmazuel commented Jul 26, 2019

So CI, is doing something really weird:

  • All Python 3.5 tests are passing for "identity" CI
  • Python 3.5 Windows passes for all tests, but linux fails for "identity" tests
  • Everything else passes in all platform and Python

I believe it's an issue in the CI :(. Re-running just the job (sigh)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Azure.Core Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants