-
Notifications
You must be signed in to change notification settings - Fork 19
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
Clearer logs and small refactoring #73
Clearer logs and small refactoring #73
Conversation
Needs converting to github actions first. Also at least one pytype fix conflicts with flake8 :-( |
dc4f07b
to
2d7c246
Compare
Ok, now it passes locally:
|
17e60b1
to
d4396ea
Compare
pglookout/cluster_monitor.py
Outdated
f_result = None | ||
result = {"fetch_time": get_iso_timestamp(), "connection": False} | ||
if not db_conn: | ||
db_conn = self._connect_to_db(instance, self.config["remote_conns"].get(instance)) | ||
if not db_conn: | ||
return result | ||
phase = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit. I would instead move the next phase = "querying status from"
out of the try, since the assignment won't fail (and if it did, it wouldn't fail with an exception type that is part of the except clause).
The logs were always showing "We still have some connected masters: {}, not failing over" which is confusing to the user as the real reason (not yet expired timeout) was hidden.
The names were misleading as they implied that only the standbys were queries, but actually all cluster members were queried
Nothing was every reading from it and it is confusing to rely on dict references to get everything updated
d4396ea
to
f21173b
Compare
f21173b
to
d6f32ab
Compare
d6f32ab
to
defb2ce
Compare
No description provided.