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 user plugin changes in check mode #596

Merged
merged 31 commits into from
Aug 30, 2024

Conversation

francescsanjuanmrf
Copy link
Contributor

@francescsanjuanmrf francescsanjuanmrf commented Nov 20, 2023

SUMMARY

The user module makes changes to the user's plugin parameter when plugin_auth_string and plugin_hash_string are not defined and is running in check mode.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

mysql_user_module

Copy link

codecov bot commented Nov 20, 2023

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 64.74%. Comparing base (37a718c) to head (2e3c9ab).

Files Patch % Lines
plugins/module_utils/user.py 0.00% 2 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (37a718c) and HEAD (2e3c9ab). Click for more details.

HEAD has 11 uploads less than BASE
Flag BASE (37a718c) HEAD (2e3c9ab)
integration 12 1
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #596       +/-   ##
===========================================
- Coverage   76.31%   64.74%   -11.57%     
===========================================
  Files          20       17        -3     
  Lines        2651     2598       -53     
  Branches      676      672        -4     
===========================================
- Hits         2023     1682      -341     
- Misses        427      697      +270     
- Partials      201      219       +18     
Flag Coverage Δ
integration 64.74% <0.00%> (-10.93%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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.

@francescsanjuanmrf hello, thanks for the fix!
Could you please:

How to run them locally, see the quickstart guide or you can just push the changes and you'll see the result in the PR. If any questions, please let us know.

Thanks

plugins/module_utils/user.py Outdated Show resolved Hide resolved
@Andersson007
Copy link
Collaborator

hello, any updates on this?

@Andersson007
Copy link
Collaborator

@francescsanjuanmrf hello, thanks for making time to work on this!
Please ignore the Sanity failure - it's unrelated to the PR.
The Integration tests for MySQL should be fixed though

@francescsanjuanmrf
Copy link
Contributor Author

francescsanjuanmrf commented Aug 26, 2024

Hello!
Sorry, I have completely forgotten about this.
I'm trying to understand why tests only fail in MySQL and not in mariadb.

@francescsanjuanmrf
Copy link
Contributor Author

Now the tests are fixed.

@Andersson007
Copy link
Collaborator

@francescsanjuanmrf if you rebase the PR (use main instead of devel mentioned in the guide), the sanity tests will get green too

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.

Anyway, the sanity failures are unrelated and is not an obstacle to get it merged.
@laurent-indermuehle and others, any suggestions or we can merge?

@francescsanjuanmrf
Copy link
Contributor Author

francescsanjuanmrf commented Aug 29, 2024

Rebase done. Sanity checks passed 👍

@Andersson007 Andersson007 merged commit 0de9685 into ansible-collections:main Aug 30, 2024
47 checks passed
@Andersson007
Copy link
Collaborator

@francescsanjuanmrf thanks for the contribution!
Feel free to join our team on the forum https://forum.ansible.com/g/MySQLTeam if you want to get more involved!

@francescsanjuanmrf
Copy link
Contributor Author

Thank you for your help!

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.

2 participants