[master] Fix 63991: Add option to use a fresh connection for mysql cache #63993
Conversation
salt-project-bot-prod-environment
bot
changed the title
bluesliverx
force-pushed
the
patch-mysql
branch
2 times, most recently
from
e1a3361 to
86be273
Compare
|
Not sure how to rerun the tests, so I just amended the commit and repushed. I cannot see how what I did could cause failures to git checkout, outright failures to run the job, etc. |
don't think it was you. GitHub actions was having some problems earlier in the day. |
cmcmarrow
left a comment
cmcmarrow
left a comment
There was a problem hiding this comment.
Thank you for the PR, Just one request.
| try: | ||
| # MySQLdb import failed, try to import PyMySQL | ||
| import pymysql | ||
| from pymysql.err import InterfaceError |
There was a problem hiding this comment.
Rather than making a throw away InterfaceError can we just wrap from pymysql.err import InterfaceError in its own try catch before we init the other stuff?
There was a problem hiding this comment.
I could, but then how would I reference InterfaceError below where it is caught without creating the class? I guess I'm not seeing what it buys us, but I'm probably missing something :)
|
@cmcmarrow I responded to your request. I could use some more details on what exactly you were looking for. |
|
@cmcmarrow still looking for another review/response to my questions. |
|
Can anyone help out with a review since @cmcmarrow seems to be MIA? |
|
@Ch3LL I'm only pinging you because you seem to the only one consistent at responding, any chance we can get this reassigned? I've been waiting for a response from @cmcmarrow for a couple of months... I'm not in a huge hurry, but might be nice to get this in before salt 3007. |
Ch3LL
left a comment
Ch3LL
left a comment
There was a problem hiding this comment.
This looks good to me, but I would like to get @garethgreenaway 's sign off as well since he has been in the mysql code the most.
5 participants
What does this PR do?
Adds a fresh connection variable for mysql_cache and fixes some other small issues that we've encountered in production at scale.
What issues does this PR fix or reference?
Fixes: #63991
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
No
Please review Salt's Contributing Guide for best practices.
See GitHub's page on GPG signing for more information about signing commits with GPG.