cratedb_toolkit/admin/xmover/model.py (1)

61-61:Standardize “no partition” sentinel.

Tests use the string "NULL". Prefer None to represent “no partition” at the data layer to avoid leaking sentinel strings into UIs. If "NULL" must be retained for compatibility, document it on the field.

doc/admin/xmover/handbook.md (1)

271-287:Add language to fenced code block to satisfy markdownlint.

Label the sample output block as text.

Apply:

-**Sample Output:**-```+**Sample Output:**+```text Found 3 shards with problematic translogs: ... Total: 3 ALTER commands generated-```+```

cratedb_toolkit/admin/xmover/util/format.py (1)

46-58:Fix typing: avoid implicit Optional (RUF013).

Annotatepartition_values as Optional and import it.

Apply:

+from typing import Optional@@-def format_table_display_with_partition(schema_name: str, table_name: str, partition_values: str = None) -> str:+def format_table_display_with_partition(+    schema_name: str, table_name: str, partition_values: Optional[str] = None+) -> str:

Optional (defensive): treat common sentinels as “no partition” to keep displays clean:

-    if partition_values:+    if partition_values and partition_values.strip() not in {"NULL", "-", "none", ""}:         return f"{base_display} {partition_values}"

cratedb_toolkit/admin/xmover/cli.py (2)

480-493:Catching broad Exception — either narrow or document.

Ifexecute_query raises a known exception type, catch that. Otherwise, suppress the Ruff BLE001 warning explicitly.

Apply one of:

-                    except Exception as e:+                    # Broad except is intentional for CLI UX+                    except Exception as e:  # noqa: BLE001

or, if available:

-                    except Exception as e:+                    except (click.ClickException, RuntimeError) as e:

Confirm the concrete exception type thrown byCrateDBClient.execute_query and replaceException accordingly.

---

506-510:Same: broad except for analysis block.

Mirror the decision above for consistency.

Apply:

-    except Exception as e:+    except Exception as e:  # noqa: BLE001

tests/admin/test_problematic_translogs.py (2)

156-159:Patch time.sleep at the module where it’s used to avoid cross-test bleed.

Patch the symbol in the CLI module (or the module that calls sleep), not the globaltime.sleep.

-), patch("time.sleep"):  # Mock sleep to speed up test+), patch("cratedb_toolkit.admin.xmover.cli.time.sleep"):  # Mock sleep where used

If sleep is called from another module, target that module path instead.

---

128-131:Make the query assertion resilient to quoting of "primary".

The SQL should quote the column assh."primary" = FALSE. Update the assertion to allow both.

+import re ...-assert "primary = FALSE" in query+assert re.search(r'"?primary"?\s*=\s*FALSE', query), query

cratedb_toolkit/admin/xmover/operational/monitor.py (1)

140-147:Use the shared formatter for table+partition display for consistency.

Align_format_recovery_table with the rest of the file.

-# Format table name with partition values if available-table_display = f"{recovery.schema_name}.{recovery.table_name}"-if recovery.partition_values:-    table_display = f"{table_display} {recovery.partition_values}"+table_display = format_table_display_with_partition(+    recovery.schema_name, recovery.table_name, recovery.partition_values+)

cratedb_toolkit/admin/xmover/util/database.py (1)

516-573:Docstring vs filter mismatch: exclude actively recovering shards or adjust wording.

get_problematic_shards says “aren't actively recovering” but currently includes allstate != 'STARTED', which can includeRECOVERING. Either update the filter or the docstring.

Option A (exclude recovering):

- where_conditions = ["s.state != 'STARTED'"]+ where_conditions = ["s.state NOT IN ('STARTED','RECOVERING')"]

Option B: Clarify the docstring to say it returns “non-started shards (may include recovering),” and rely on caller-side filtering (as done in monitor).

cratedb_toolkit/admin/xmover/analysis/shard.py (2)

1069-1127:Narrow the try/except and useelse: for Ruff TRY300; annotate broad catch if intentional.

Limit the try to the query, put the rendering inelse:, and annotate the broad catch (BLE001) since this is a CLI UX path.

-        try:-            result = self.client.execute_query(query, [size_mb])-            rows = result.get("rows", [])--            if not rows:-                console.print(f"[green]✓ No replica shards found with translog uncommitted size > {size_mb}MB[/green]")-                return []--            console.print(f"Found {len(rows)} shards with problematic translogs:")-            console.print()-            ...-            return alter_commands-        except Exception as e:-            console.print(f"[red]Error analyzing problematic translogs: {e}[/red]")+        try:+            result = self.client.execute_query(query, [size_mb])+        except Exception as e:  # noqa: BLE001+            console.print(f"[red]Error analyzing problematic translogs: {e}[/red]")             import traceback-             console.print(f"[dim]{traceback.format_exc()}[/dim]")             return []+        else:+            rows = result.get("rows", [])+            if not rows:+                console.print(f"[green]✓ No replica shards found with translog uncommitted size > {size_mb}MB[/green]")+                return []+            console.print(f"Found {len(rows)} shards with problematic translogs:")+            console.print()+            ...+            return alter_commands

---

771-773:Remove unused variable or use it.

zone is computed but unused.

-# FIXME: `zone` does not get used.-zone = next((node.zone for node in self.nodes if node.id == node_id), "unknown")  # noqa: F841+# zone lookup omitted intentionally; remove variable to avoid noise

Or use it in emitted stats if valuable.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: amotlPR: crate/cratedb-toolkit#523File: cratedb_toolkit/admin/xmover/analyze/shard.py:0-0Timestamp: 2025-09-18T17:32:10.537ZLearning: XMover (cratedb_toolkit.admin.xmover) intentionally uses print statements throughout its codebase instead of structured logging, as it's designed as a CLI tool where direct console output is the intended behavior.