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

Change deprecated ssl option in aiohttp #1155

Merged
merged 2 commits into from
Nov 14, 2018
Merged

Change deprecated ssl option in aiohttp #1155

merged 2 commits into from
Nov 14, 2018

Conversation

nirgn975
Copy link
Contributor

Change verify_ssl to ssl in aiohttp.TCPConnector, because verify_ssl is now deprecated.

Copy link
Contributor

@r0fls r0fls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not currently using a new enough version for that argument: https://github.com/channelcat/sanic/blob/master/requirements-dev.txt#L2

@hatarist
Copy link
Contributor

hatarist commented May 1, 2018

@r0fls but aiohttp>=2.3.0 (the one that's currently in the requierements-dev.txt you gave the link to) at the moment installs the 3.1.3; maybe it's worth locking the version at ==2.3.0 or the latest 2.x one, ==2.3.10?

@r0fls
Copy link
Contributor

r0fls commented May 1, 2018

Ah thanks @hatarist, we could also pin the later version since 3.x should work fine.

@r0fls
Copy link
Contributor

r0fls commented May 1, 2018

Or we could just merge this, but if the version is actually 2.x it will fail so it seems we should bump the requirements to me

@aviv-ebates
Copy link

mmm.... Since the aiohttp is only mentioned in requierements-dev.txt, user code isn't bound by it; That means that my code is getting the deprecation error when I run my tests.
Do you know if there's a reason not to just bump to aiohttp 3.0?

@ahopkins
Copy link
Member

@nirgn975 Can you update your branch?

@codecov-io
Copy link

codecov-io commented Nov 13, 2018

Codecov Report

Merging #1155 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1155      +/-   ##
==========================================
+ Coverage   83.39%   83.45%   +0.05%     
==========================================
  Files          17       17              
  Lines        1704     1704              
  Branches      322      322              
==========================================
+ Hits         1421     1422       +1     
  Misses        217      217              
+ Partials       66       65       -1
Impacted Files Coverage Δ
sanic/testing.py 94.8% <100%> (ø) ⬆️
sanic/router.py 92.69% <0%> (+0.45%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 296cda7...8711c61. Read the comment docs.

@nirgn975
Copy link
Contributor Author

nirgn975 commented Nov 13, 2018

@nirgn975 Can you update your branch?

@ahopkins of course, did it and travis-ci is passed.

@ahopkins
Copy link
Member

Thanks @nirgn975!

@sjsadowski sjsadowski merged commit efb9a42 into sanic-org:master Nov 14, 2018
@uda
Copy link

uda commented Dec 16, 2018

When will this be published to pypi?

@ahopkins
Copy link
Member

@uda Should be this week 🍻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants