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

Adds support for show binary log status statement #638

Conversation

dennisurtubia
Copy link
Contributor

SUMMARY

Adds support for show binary log status statement.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

mysql_replication

ADDITIONAL INFORMATION

SHOW MASTER STATUS was replaced by SHOW BINARY LOG STATUS. Doc

@dennisurtubia dennisurtubia marked this pull request as draft May 28, 2024 01:04
@dennisurtubia dennisurtubia force-pushed the feat/get-primary-mysql-8.4 branch 3 times, most recently from 6615877 to 71c2691 Compare May 29, 2024 00:26
@dennisurtubia dennisurtubia marked this pull request as ready for review May 29, 2024 01:52
@dennisurtubia
Copy link
Contributor Author

Feature asked on #635 (comment)

@dennisurtubia
Copy link
Contributor Author

dennisurtubia commented May 29, 2024

@Andersson007 @laurent-indermuehle I tried to implement an integration test checking the executed query, but it don't works, seems that getprimary mode never returns executed queries.

I think the implemented integration tests are sufficiently to test getprimary mode, however, I think it's important to configure in our actions pipeline to execute MySQL >= 8.2.0

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.

@dennisurtubia LGTM, thanks for the contribution?

@laurent-indermuehle please take a look

@eRadical
Copy link
Contributor

eRadical commented May 30, 2024

Hi @dennisurtubia,

If we're here can we also extend for MariaDB >= 10.5.2 with:

if get_server_implementation(cursor) == "mariadb" and LooseVersion(version) >= LooseVersion("10.5.2"):
        term = "BINLOG"

Ref.: SHOW BINLOG STATUS

Regards!

@dennisurtubia dennisurtubia force-pushed the feat/get-primary-mysql-8.4 branch 2 times, most recently from ae2a77d to b8c7eca Compare May 30, 2024 12:53
@dennisurtubia dennisurtubia force-pushed the feat/get-primary-mysql-8.4 branch from b8c7eca to 351e031 Compare May 30, 2024 12:54
Copy link

codecov bot commented May 30, 2024

Codecov Report

Attention: Patch coverage is 55.55556% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 71.07%. Comparing base (a80b805) to head (b8c7eca).

Current head b8c7eca differs from pull request most recent head 351e031

Please upload reports for the commit 351e031 to get more accurate results.

Files Patch % Lines
plugins/modules/mysql_replication.py 55.55% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #638       +/-   ##
===========================================
+ Coverage   24.40%   71.07%   +46.67%     
===========================================
  Files          28       15       -13     
  Lines        2639     2465      -174     
  Branches      661      635       -26     
===========================================
+ Hits          644     1752     +1108     
+ Misses       1935      493     -1442     
- Partials       60      220      +160     
Flag Coverage Δ
integration 71.07% <55.55%> (?)
sanity ?
units ?

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.

@eRadical great feedback, approved the PR again, thanks! waiting for @laurent-indermuehle 's feedback

@dennisurtubia
Copy link
Contributor Author

Hi @dennisurtubia,

If we're here can we also extend for MariaDB >= 10.5.2 with:

if get_server_implementation(cursor) == "mariadb" and LooseVersion(version) >= LooseVersion("10.5.2"):
        term = "BINLOG"

Ref.: SHOW BINLOG STATUS

Regards!

Hey, @eRadical thank you for your suggestion! Done.

@laurent-indermuehle
Copy link
Collaborator

The code starts to be really beautiful! Thanks for your contributions @eRadical and @dennisurtubia!

@laurent-indermuehle laurent-indermuehle merged commit 4761034 into ansible-collections:main May 30, 2024
43 checks passed
@dennisurtubia dennisurtubia deleted the feat/get-primary-mysql-8.4 branch May 30, 2024 15:18
@Andersson007
Copy link
Collaborator

@dennisurtubia thanks a lot for the improvement!
@eRadical @laurent-indermuehle thanks for reviewing!

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.

4 participants