Skip to content

Conversation

@paulcampbell-payroc
Copy link
Contributor

SUMMARY

Galera wsrep global variables sometimes use uppercase identifiers (e.g. wsrep_OSU_method) which causes them to fail validation against the existing regex. While it is possible to refer to these vars with their lowercase equivalents to make them work, this would be more confusing than changing the regex to support legal characters.

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.

@pcampbell-payroc hello, thanks for the PR!

  • Could you please add a changelog fragment (under bugfix i think) so that our users will see the change?
  • We have an official room on Matrix to share experience, welcome to join!

@paulcampbell-payroc
Copy link
Contributor Author

I hope that's in the correct format for the project.

Comment on lines 2 to 6
bugfix:
- mysql_variables - Add uppercase character pattern to regex to allow GLOBAL
variables containing uppercase characters.
This recognises variable names used in Galera, e.g. wsrep_OSU_method,
which break the normal pattern of all lowercase characters.
Copy link
Collaborator

@Andersson007 Andersson007 Feb 2, 2023

Choose a reason for hiding this comment

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

Suggested change
bugfix:
- mysql_variables - Add uppercase character pattern to regex to allow GLOBAL
variables containing uppercase characters.
This recognises variable names used in Galera, e.g. wsrep_OSU_method,
which break the normal pattern of all lowercase characters.
bugfixes:
- mysql_variables - add uppercase character pattern to regex to allow GLOBAL
variables containing uppercase characters.
This recognizes variable names used in Galera, for example, ``wsrep_OSU_method``,
which breaks the normal pattern of all lowercase characters (https://github.com/ansible-collections/community.mysql/pull/501).

Copy link
Collaborator

@Andersson007 Andersson007 Feb 2, 2023

Choose a reason for hiding this comment

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

sorry i made a mistake in the key
bugfix -> bugfixes (this will fix the sanity errors)
Also fyi: I'll be on Fosdem and if nobody merges it before I'll merge this at the end of next week

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please make any changes you think are suitable. I only started working with Ansible last week so I'm really happy you were even considering accepting the change.

@Andersson007
Copy link
Collaborator

@pcampbell-payroc

I only started working with Ansible last week so I'm really happy you were even considering accepting the change.

You're a quintessence of contributor! :) It's so cool to see people giving back so fast!

@Andersson007 Andersson007 merged commit 521443a into ansible-collections:main Feb 4, 2023
@Andersson007
Copy link
Collaborator

@pcampbell-payroc thank you for the contribution!

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