Skip to content

Commit

Permalink
mysql_legacy: fix pymysql queries
Browse files Browse the repository at this point in the history
* Fix `set_master_use_gtid()` query, its value it's part of the syntax,
  avoid pymysql quoting it.
* Fix query formatting in `set_replication_parameters()`.
* Fix check in `replication_lag()` that would raise if the lag is 0.0s.

Change-Id: I2a02f98dbbbce8c65edbfe9383a263d028fa9193
  • Loading branch information
volans- committed Nov 14, 2024
1 parent 5cbd021 commit cf0c23b
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 18 deletions.
17 changes: 9 additions & 8 deletions spicerack/mysql_legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,8 @@ def set_master_use_gtid(self, setting: MasterUseGTID) -> None:
if not isinstance(setting, MasterUseGTID):
raise MysqlLegacyError(f"Only instances of MasterUseGTID are accepted, got: {type(setting)}")

self.execute("CHANGE MASTER TO MASTER_USE_GTID=%s", [setting.value])
# Not using placeholder replacements ad MariaDB requires it to be a syntax word and raises if it's quoted
self.execute(f"CHANGE MASTER TO MASTER_USE_GTID={setting.value}")

def stop_mysql(self) -> Iterator[tuple[NodeSet, MsgTreeElem]]:
"""Stops mariadb service.
Expand Down Expand Up @@ -614,13 +615,13 @@ def set_replication_parameters(self, *, replication_info: ReplicationInfo, user:
"""Sets the replication parameters for the MySQL instance."""
self.execute(
(
"CHANGE MASTER TO master_host=%s(primary), "
"master_port=%s(port), "
"CHANGE MASTER TO master_host=%(primary)s, "
"master_port=%(port)s, "
"master_ssl=1, "
"master_log_file=%s(binlog), "
"master_log_pos=%s(position), "
"master_user=%s(user), "
"master_password=%s(password)"
"master_log_file=%(binlog)s, "
"master_log_pos=%(position)s, "
"master_user=%(user)s, "
"master_password=%(password)s"
),
{
"primary": replication_info.primary,
Expand Down Expand Up @@ -689,7 +690,7 @@ def replication_lag(self) -> Decimal:
if row is None:
raise MysqlLegacyError("The replication lag query returned no data")

if not row["lag"]:
if "lag" not in row or row["lag"] is None:
raise MysqlLegacyError(f"Unable to get lag information from: {row}")

return row["lag"]
Expand Down
25 changes: 15 additions & 10 deletions spicerack/tests/unit/test_mysql_legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -319,8 +319,7 @@ def test_set_master_use_gtid_ok(self, setting, expected):
"""It should execute MASTER_USE_GTID with the given value."""
self.mocked_cursor.execute.return_value = 0
self.single_instance.set_master_use_gtid(setting)
query = "CHANGE MASTER TO MASTER_USE_GTID=%s"
self.mocked_cursor.execute.assert_any_call(query, [expected])
self.mocked_cursor.execute.assert_any_call(f"CHANGE MASTER TO MASTER_USE_GTID={expected}", None)

def test_set_master_use_gtid_invalid(self):
"""It should raise MysqlLegacyError if called with an invalid setting."""
Expand Down Expand Up @@ -463,13 +462,13 @@ def test_set_replication_parameters(self):
call_args = self.mocked_cursor.execute.call_args_list[0].args
for part in (
"CHANGE MASTER TO",
"master_host=%s(primary)",
"master_port=%s(port)",
"master_host=%(primary)s",
"master_port=%(port)s",
"master_ssl=1",
"master_log_file=%s(binlog)",
"master_log_pos=%s(position)",
"master_user=%s(user)",
"password=%s(password)",
"master_log_file=%(binlog)s",
"master_log_pos=%(position)s",
"master_user=%(user)s",
"password=%(password)s",
):
assert part in call_args[0]

Expand Down Expand Up @@ -574,10 +573,16 @@ def test_replication_lag_no_data(self):

self.mocked_cursor.fetchone.assert_not_called()

def test_replication_lag_no_lag(self):
@pytest.mark.parametrize(
"row",
(
{"lag": None},
{"invalid": Decimal(0.1234)},
),
)
def test_replication_lag_no_lag(self, row):
"""It should raise a MysqlLegacyError if the returned lag is None."""
self.mocked_cursor.execute.side_effect = [1, 0]
row = {"lag": None}
self.mocked_cursor.fetchone.return_value = row
error_message = f"Unable to get lag information from: {row}"

Expand Down

0 comments on commit cf0c23b

Please sign in to comment.