Skip to content

Commit 94415a0

Browse files
committed
fix: address review — single timestamp, descriptive URL error, skipped count
- Capture single datetime.now() in Registry.update() and reuse for both installed_at and updated_at to avoid tiny time skews - Include ValueError message in --from URL validation error so users see the actual failure reason (e.g., missing hostname) - Track skipped workflow count in 'workflow update' and show 'No updates found (N skipped)' instead of misleading 'All workflows are up to date!' when workflows were skipped
1 parent 9d8de6e commit 94415a0

2 files changed

Lines changed: 16 additions & 6 deletions

File tree

src/specify_cli/__init__.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5028,9 +5028,9 @@ def _validate_and_install_local(yaml_path: Path, source_label: str) -> None:
50285028
if from_url:
50295029
try:
50305030
_validate_url_scheme(from_url)
5031-
except ValueError:
5032-
console.print("[red]Error:[/red] URL must use HTTPS for security.")
5033-
console.print("HTTP is only allowed for localhost URLs.")
5031+
except ValueError as exc:
5032+
console.print(f"[red]Error:[/red] Invalid URL: {exc}")
5033+
console.print("URL must use HTTPS for security. HTTP is only allowed for localhost URLs.")
50345034
raise typer.Exit(1)
50355035

50365036
console.print("[yellow]Warning:[/yellow] Installing from external URL.")
@@ -5365,16 +5365,19 @@ def workflow_update(
53655365

53665366
updates_available = []
53675367
catalog_errors = 0
5368+
skipped = 0
53685369
for wf_id in workflows_to_update:
53695370
metadata = installed.get(wf_id, {})
53705371
if not isinstance(metadata, dict):
53715372
console.print(f"⚠ {wf_id}: Malformed workflow registry entry (skipping)")
5373+
skipped += 1
53725374
continue
53735375
installed_ver_str = str(metadata.get("version", "0.0.0"))
53745376
try:
53755377
installed_version = pkg_version.Version(installed_ver_str)
53765378
except (pkg_version.InvalidVersion, TypeError):
53775379
console.print(f"⚠ {wf_id}: Invalid installed version '{installed_ver_str}' (skipping)")
5380+
skipped += 1
53785381
continue
53795382

53805383
try:
@@ -5386,17 +5389,20 @@ def workflow_update(
53865389

53875390
if not cat_info:
53885391
console.print(f"⚠ {wf_id}: Not found in catalog (skipping)")
5392+
skipped += 1
53895393
continue
53905394

53915395
if not cat_info.get("_install_allowed", True):
53925396
console.print(f"⚠ {wf_id}: Updates not allowed from '{cat_info.get('_catalog_name', 'catalog')}' (skipping)")
5397+
skipped += 1
53935398
continue
53945399

53955400
cat_ver_str = str(cat_info.get("version", "0.0.0"))
53965401
try:
53975402
catalog_version = pkg_version.Version(cat_ver_str)
53985403
except (pkg_version.InvalidVersion, TypeError):
53995404
console.print(f"⚠ {wf_id}: Invalid catalog version '{cat_ver_str}' (skipping)")
5405+
skipped += 1
54005406
continue
54015407

54025408
if catalog_version > installed_version:
@@ -5416,7 +5422,10 @@ def workflow_update(
54165422
if catalog_errors:
54175423
console.print(f"\n[yellow]Could not check {catalog_errors} workflow(s) due to catalog errors[/yellow]")
54185424
raise typer.Exit(1)
5419-
console.print("\n[green]All workflows are up to date![/green]")
5425+
if skipped:
5426+
console.print(f"\n[green]No updates found[/green] ({skipped} workflow(s) skipped)")
5427+
else:
5428+
console.print("\n[green]All workflows are up to date![/green]")
54205429
raise typer.Exit(0)
54215430

54225431
console.print("\n[bold]Updates available:[/bold]\n")

src/specify_cli/workflows/catalog.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,13 @@ def update(self, workflow_id: str, fields: dict[str, Any]) -> None:
165165
import copy
166166
safe_fields = copy.deepcopy(fields)
167167
safe_fields.pop("installed_at", None) # always preserve original
168+
now = datetime.now(timezone.utc).isoformat()
168169
existing.update(safe_fields)
169170
if installed_at is not None:
170171
existing["installed_at"] = installed_at
171172
if "installed_at" not in existing:
172-
existing["installed_at"] = datetime.now(timezone.utc).isoformat()
173-
existing["updated_at"] = datetime.now(timezone.utc).isoformat()
173+
existing["installed_at"] = now
174+
existing["updated_at"] = now
174175
self.save()
175176

176177
def get(self, workflow_id: str) -> dict[str, Any] | None:

0 commit comments

Comments
 (0)