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

Added missing gtid_port to proxysql_backend_servers module #118

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thanos-k
Copy link

@thanos-k thanos-k commented Oct 16, 2022

SUMMARY

Added missing gtid_port to proxysql_backend_servers module

Fixes #117

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

modules/proxysql_backend_servers

Comment on lines +34 to +37
gtid_port:
description:
- The backend server port where ProxySQL Binlog Reader listens on for GTID tracking
type: int
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
gtid_port:
description:
- The backend server port where ProxySQL Binlog Reader listens on for GTID tracking
type: int
gtid_port:
description:
- The backend server port where ProxySQL Binlog Reader listens on for GTID tracking
type: int
version_added: 1.5.0

It should also be mentioned in the description, that the default value is 0, even if it's not set by the ansible module.

gtid_port INT CHECK ((gtid_port <> port OR gtid_port=0) AND gtid_port >= 0 AND gtid_port <= 65535) NOT NULL DEFAULT 0,

@markuman
Copy link
Member

@thanos-k Thanks for the quick PR.
We need also an integration test, that verified that the parameter is working correctly. Mainly test to change the gtid_port of an existing row/record.

You can try to expand the existing test: https://github.com/ansible-collections/community.proxysql/tree/main/tests/integration/targets/test_proxysql_backend_servers/tasks, but it's difficult to see through.
You can also just add a new taskfile in tests/integration/targets/test_proxysql_backend_servers/ and include it at the end of the main.yml

@markuman markuman mentioned this pull request Oct 17, 2022
Copy link
Contributor

@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.

@thanos-k thanks for the PR!

Comment on lines +1 to +2
bugfixes:
- modules/proxysql_backend_servers - Added missing gtid_port to proxysql_backend_servers module (https://github.com/ansible-collections/community.proxysql/pull/118).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
bugfixes:
- modules/proxysql_backend_servers - Added missing gtid_port to proxysql_backend_servers module (https://github.com/ansible-collections/community.proxysql/pull/118).
minor_changes:
- proxysql_backend_servers - Added missing ``gtid_port`` argument to the module (https://github.com/ansible-collections/community.proxysql/pull/118).

@thanos-k @markuman I think it should be of the minor_changes: category as it adds a new argument.

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.

Support gtid_port in proxysql_backend_servers
3 participants