-
Notifications
You must be signed in to change notification settings - Fork 258
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
SSL Support (plus mysql_clear_password plugin for RDS) #280
Conversation
Codecov Report
@@ Coverage Diff @@
## master #280 +/- ##
=======================================
Coverage 92.93% 92.93%
=======================================
Files 9 9
Lines 1161 1161
Branches 173 173
=======================================
Hits 1079 1079
Misses 53 53
Partials 29 29 Continue to review full report at Codecov.
|
It passes travis, merge merge merge!!! :D |
I will try to review tonight, have lots of thing to do this days after work.
…On Thu, Apr 19, 2018, 02:43 Terry Cain ***@***.***> wrote:
It passes travis, merge merge merge!!! :D
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#280 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AANoZ8vZPj1NpnJKUwe5mOvqr88eowCgks5tp8-zgaJpZM4Ta7lS>
.
|
@terrycain you did very good job! PR concise and elegant! |
New version available on PyPI https://pypi.org/project/aiomysql/0.0.13/ |
@jettify No problem, was easy after learning that trick with passing in an active socket to open_connection. Will pr again in a bit to add SSL example + an example of how to use it with RDS's token thing. Onto other things, mind checking out my aiobotocore pr 😉 |
I'll be user number 2 (ha ha joke's on me)! This is awesome! I really want to use aioboto but it needs to support RDS auth tokens KMS and SNS. |
@terrisgit It probably already does, raise some issues and I'll get cracking |
This implementation of SSL is failing a lot when attempting to run SQL commands. I am still investigating. |
Hmm not good, if you can find some repeatable code that will trigger it I'll get on that this weekend |
Can you share traceback?
…On Fri, Apr 20, 2018, 20:25 Terry Cain ***@***.***> wrote:
Hmm not good, if you can find some repeatable code that will trigger it
I'll get on that this weekend
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#280 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AANoZ_bkaRsOMNvUX3NvsOPKz489AMhCks5tqhoZgaJpZM4Ta7lS>
.
|
ahh so its fine until you try and kill a connection? |
Ok, i'll spin up something to see if i can reproduce |
Ok very weird, if I run the following in debug mode under pycharm, it works fine 100% of the time. @jettify @terrisgit Any ideas why it would work whilst debugging and not when running normally? import asyncio
import ssl
import aiomysql
async def main(loop):
ctx = ssl.SSLContext(ssl.PROTOCOL_TLSv1)
ctx.check_hostname = False
ctx.load_verify_locations(cafile='../tests/ssl_resources/ssl/ca.pem')
print('pre_pool')
pool = await aiomysql.create_pool(
host='localhost',
port=3308,
user='root',
password='root',
loop=loop, ssl=ctx
)
print('start')
await tls_query(pool)
print('end')
async def tls_query(pool):
async with pool.get() as conn:
async with conn.cursor() as cur:
# Run simple command
await cur.execute("SHOW DATABASES;")
value = await cur.fetchall()
print(value)
# Check TLS variables
await cur.execute("SHOW STATUS LIKE '%Ssl_version%';")
value = await cur.fetchone()
print(value[1])
event_loop = asyncio.get_event_loop()
event_loop.run_until_complete(main(event_loop)) Traceback:
|
Control-C was a red herring. The problem is related, as you suspected, to closing a connection pool when a connection is in use. The following hangs or fails in #280 but not in #225 . Sorry it's not minimal but I'm sure you can poke at it and find a simpler version.
|
Can you please check this patch? #282 |
@jettify That patch looks like it did the trick 😄 @terrisgit Looks like the patch #282 seems to fix your example, I dont see any bad descriptor exceptions |
Yes, that fixed it! Hope you release it soon. |
Git closed last one when i force pushed.
Long discussion in #225
@jettify @terrisgit - new PR, less code :D
Also credit to Alex as I made a test where MySQL then tried to renegotiate the auth plugin, so I imported his
process_auth
function (and github realises it was him 😄)@jettify whats next.