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

fix connection arguments for MySQLdb <2.0 !=1.0 #553

Conversation

betanummeric
Copy link
Member

fixes #546 and #551

@betanummeric betanummeric marked this pull request as ready for review May 23, 2023 10:02
@betanummeric betanummeric requested a review from Andersson007 May 23, 2023 10:03
@codecov
Copy link

codecov bot commented May 23, 2023

Codecov Report

Merging #553 (218f854) into main (bff05ce) will decrease coverage by 2.32%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #553      +/-   ##
==========================================
- Coverage   76.67%   74.35%   -2.32%     
==========================================
  Files          28       18      -10     
  Lines        2379     2254     -125     
  Branches      563      560       -3     
==========================================
- Hits         1824     1676     -148     
- Misses        395      407      +12     
- Partials      160      171      +11     
Flag Coverage Δ
integration 73.73% <0.00%> (+0.26%) ⬆️
sanity ?
units ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
plugins/module_utils/mysql.py 62.50% <100.00%> (+4.16%) ⬆️

... and 18 files with indirect coverage changes

@betanummeric
Copy link
Member Author

The CI errors appear to be unrelated to the change:

ERROR! [DEPRECATED]: ansible.builtin.include has been removed. Use include_tasks or import_tasks instead. This feature was removed from ansible-core in a release after 2023-05-16. Please update your playbooks.

occurring at: https://github.com/betanummeric/community.mysql/blob/bff05ce8ddb99f53270ad11e753c153df604adb5/tests/integration/targets/test_mysql_variables/tasks/mysql_variables.yml#L40

Copy link
Collaborator

@Andersson007 Andersson007 left a comment

Choose a reason for hiding this comment

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

@betanummeric feel free to merge it as is or fix the unrelated line and merge it after, as you prefer

@Andersson007
Copy link
Collaborator

oh, it seems to be not as streightforward as we thought. maybe as there are still not a lot of unrelated-to-this-fix change maybe revert last to commits and merge it as it was originally? (just a suggestion, if you like we could merge all stuff here)

@betanummeric
Copy link
Member Author

It appears to be just syntax errors, but in many places, hence tedious to fix. Let's fix that in another PR, I will revert the attempt to fix it here.

@betanummeric betanummeric force-pushed the fix_database_connection_argument2 branch from 0f1d1d8 to 218f854 Compare May 23, 2023 13:31
@betanummeric betanummeric merged commit b6ad472 into ansible-collections:main May 23, 2023
@betanummeric betanummeric deleted the fix_database_connection_argument2 branch May 23, 2023 13:32
@Andersson007
Copy link
Collaborator

It appears to be just syntax errors, but in many places, hence tedious to fix. Let's fix that in another PR, I will revert the attempt to fix it here.

SGTM, thanks

laurent-indermuehle pushed a commit to laurent-indermuehle/community.mysql that referenced this pull request Oct 3, 2023
)

* fix connection arguments for MySQLdb <2.0 !=1.0

* add changelog fragment

---------

Co-authored-by: Felix Hamme <felix.hamme@ionos.com>
laurent-indermuehle pushed a commit to laurent-indermuehle/community.mysql that referenced this pull request Oct 3, 2023
)

* fix connection arguments for MySQLdb <2.0 !=1.0

* add changelog fragment

---------

Co-authored-by: Felix Hamme <felix.hamme@ionos.com>
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.

Possible recurrence of issue #151 following latest release of community.mysql?
3 participants