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

Moving Package Variable to Monitor #93

Merged
merged 3 commits into from
Feb 1, 2023
Merged

Conversation

Areson
Copy link
Collaborator

@Areson Areson commented Feb 1, 2023

Moved the package-level variables statusQuery and newTerms to the monitor in case multiple monitors within the same domain are created but are running on different MySQL versions.

Added a check around replStatus when getting the "source" entry. In cases where something is not a replica the map will be nil and should not be accessed.

Moved the package-level variables `statusQuery` and `newTerms` to the monitor in case multiple monitors within the same domain are created but are running on different MySQL versions.

Added a check around `replStatus` when getting the "source" entry. In cases where something is not a replica the map will be nil and should not be accessed.
@Areson Areson requested a review from daniel-nichter February 1, 2023 01:17
@@ -86,9 +90,6 @@ func (c *Repl) Help() blip.CollectorHelp {
}
}

var statusQuery = "SHOW SLAVE STATUS" // SHOW REPLICA STATUS as of 8.022
Copy link
Member

Choose a reason for hiding this comment

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

Please put that comment back, i.e. (few lines up in the diff):

		statusQuery:     "SHOW SLAVE STATUS",  // SHOW REPLICA STATUS as of 8.022

When working with MySQL, you'll find that such "as of " comments are common and helpful, especially when your career with MySQL begins to span decades. Lots and lots of changes, and who can remember them all?

@Areson Areson merged commit 2c0839a into main Feb 1, 2023
@Areson Areson deleted the ioberst-repl-package-variables branch February 1, 2023 18:06
Areson added a commit that referenced this pull request Feb 1, 2023
Areson added a commit that referenced this pull request Feb 1, 2023
Related to #93

Updated unit tests, as now that compression is _actually_ working the client compresses the data for us in the unit test.
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