Skip to content

Commit 353e20f

Browse files
joseph-isaacsjoseph-isaacs
committed
fix
Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
1 parent b6fe892 commit 353e20f

3 files changed

Lines changed: 39 additions & 66 deletions

File tree

‎vortex-test/compat-gen/DESIGN.md‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
<!-- SPDX-License-Identifier: Apache-2.0 -->
2+
<!--SPDX-FileCopyrightText: Copyright the Vortex contributors -->
3+
14
# Vortex Backward-Compatibility Testing
25

36
## The Problem
@@ -190,9 +193,15 @@ values.
190193

191194
### Fixture upload (`.github/workflows/compat-gen-upload.yml`)
192195

193-
Used to upload fixtures for each new release.
196+
Used to upload fixtures for each new release. Triggered via **manual
197+
dispatch** with an optional `git_ref` input (defaults to HEAD).
198+
199+
Two-phase workflow:
194200

195-
Triggered via **manual dispatch** with a required `version` input.
201+
1. **dry-run** — auto-detects the version from the nearest git tag, builds
202+
fixtures, and prints what would be uploaded.
203+
2. **upload** — requires manual approval via the `compat-upload` GitHub
204+
environment, then performs the actual upload to S3.
196205

197206
### Compat validation (`.github/workflows/compat-validation.yml`)
198207

‎vortex-test/compat-gen/scripts/compat.py‎

Lines changed: 26 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,6 @@ def display_name(self) -> str:
114114
raise NotImplementedError
115115

116116

117-
118117
class LocalStore(Store):
119118
"""Fixture store backed by a local directory."""
120119

@@ -270,7 +269,9 @@ def _validate_manifest(manifest: dict, version: str) -> None:
270269
try:
271270
jsonschema.validate(manifest, MANIFEST_SCHEMA)
272271
except jsonschema.ValidationError as e:
273-
raise ValueError(f"v{version} manifest: {e.message} (at path: {'/'.join(str(p) for p in e.absolute_path)})") from e
272+
raise ValueError(
273+
f"v{version} manifest: {e.message} (at path: {'/'.join(str(p) for p in e.absolute_path)})"
274+
) from e
274275

275276

276277
def _read_manifest(store: Store, version: str) -> dict | None:
@@ -296,7 +297,10 @@ def _read_manifest(store: Store, version: str) -> dict | None:
296297

297298

298299
def _merge_manifest(
299-
store: Store, fixtures_json: dict, version: str, prev_version: str | None,
300+
store: Store,
301+
fixtures_json: dict,
302+
version: str,
303+
prev_version: str | None,
300304
) -> dict:
301305
"""Build a manifest for `version`, using sha256 from Rust-generated fixtures.json."""
302306
entries = []
@@ -423,18 +427,13 @@ def _publish_full(
423427
return
424428

425429
if not args.yes:
426-
_info(
427-
f"\nabout to upload {len(manifest['fixtures'])} fixtures "
428-
f"for v{version} to {store.display_name()}"
429-
)
430+
_info(f"\nabout to upload {len(manifest['fixtures'])} fixtures for v{version} to {store.display_name()}")
430431
answer = input("proceed? [y/N] ").strip().lower()
431432
if answer not in ("y", "yes"):
432433
_info("aborted")
433434
sys.exit(1)
434435

435-
_info(
436-
f"uploading {len(manifest['fixtures'])} fixtures to {store.display_name()}..."
437-
)
436+
_info(f"uploading {len(manifest['fixtures'])} fixtures to {store.display_name()}...")
438437
_parallel_upload(
439438
store,
440439
[(f"v{version}/arrays/{e['name']}", output / e["name"]) for e in manifest["fixtures"]],
@@ -449,10 +448,7 @@ def _publish_full(
449448
store.write("versions.json", (json.dumps(versions, indent=2) + "\n").encode())
450449
_info(" updated versions.json")
451450

452-
_info(
453-
f"\ndone: {len(manifest['fixtures'])} fixtures for v{version} "
454-
f"published to {store.display_name()}"
455-
)
451+
_info(f"\ndone: {len(manifest['fixtures'])} fixtures for v{version} published to {store.display_name()}")
456452

457453

458454
def _publish_update(
@@ -492,10 +488,7 @@ def _publish_update(
492488
if remote_data is not None:
493489
remote_hash = hashlib.sha256(remote_data).hexdigest()
494490
if local_hash != remote_hash:
495-
_info(
496-
f"error: hash mismatch for {name}: "
497-
f"local={local_hash[:12]} remote={remote_hash[:12]}"
498-
)
491+
_info(f"error: hash mismatch for {name}: local={local_hash[:12]} remote={remote_hash[:12]}")
499492
sys.exit(1)
500493
else:
501494
_info(f" {name}: unchanged (sha256 match)")
@@ -515,10 +508,7 @@ def _publish_update(
515508
return
516509

517510
if not args.yes:
518-
_info(
519-
f"\nabout to upload {len(new_fixtures)} new fixture(s) "
520-
f"for v{version} to {store.display_name()}"
521-
)
511+
_info(f"\nabout to upload {len(new_fixtures)} new fixture(s) for v{version} to {store.display_name()}")
522512
answer = input("proceed? [y/N] ").strip().lower()
523513
if answer not in ("y", "yes"):
524514
_info("aborted")
@@ -545,10 +535,7 @@ def _publish_update(
545535
store.write(f"{prefix}/manifest.json", (json.dumps(updated_manifest, indent=2) + "\n").encode())
546536
_info(" updated manifest.json")
547537

548-
_info(
549-
f"\ndone: added {len(new_fixtures)} new fixture(s) to v{version} "
550-
f"in {store.display_name()}"
551-
)
538+
_info(f"\ndone: added {len(new_fixtures)} new fixture(s) to v{version} in {store.display_name()}")
552539

553540

554541
def cmd_check(args: argparse.Namespace) -> None:
@@ -608,19 +595,14 @@ def cmd_check(args: argparse.Namespace) -> None:
608595
total_skipped += skipped
609596

610597
if failed_list:
611-
_info(
612-
f" v{version}: {passed} passed, {len(failed_list)} FAILED, "
613-
f"{skipped} skipped"
614-
)
598+
_info(f" v{version}: {passed} passed, {len(failed_list)} FAILED, {skipped} skipped")
615599
for f in failed_list:
616600
_info(f" FAIL {f['name']}: {f['error']}")
617601
all_failures.append((version, f["name"], f["error"]))
618602
else:
619603
_info(f" v{version}: {passed} passed, {skipped} skipped")
620604

621-
_info(
622-
f"\nresult: {total_passed} passed, {total_failed} failed, {total_skipped} skipped"
623-
)
605+
_info(f"\nresult: {total_passed} passed, {total_failed} failed, {total_skipped} skipped")
624606
if all_failures:
625607
sys.exit(1)
626608

@@ -734,9 +716,7 @@ def cmd_validate_manifest(args: argparse.Namespace) -> None:
734716
else:
735717
new = len(names) - len(prev_names)
736718
extra = f" (+{new} new)" if new > 0 else ""
737-
_info(
738-
f" v{prev_version} -> v{version}: ok ({len(names)} fixtures{extra})"
739-
)
719+
_info(f" v{prev_version} -> v{version}: ok ({len(names)} fixtures{extra})")
740720
else:
741721
_info(f" v{version}: {len(names)} fixtures (first)")
742722

@@ -796,9 +776,7 @@ def _cargo_run_cmd() -> list[str]:
796776
return ["cargo", "run", "-p", CARGO_BIN, "--release", "--"]
797777

798778

799-
def _run_cmd(
800-
cmd: list[str], check: bool = False, cwd: Path | None = None
801-
) -> subprocess.CompletedProcess:
779+
def _run_cmd(cmd: list[str], check: bool = False, cwd: Path | None = None) -> subprocess.CompletedProcess:
802780
_info(f" $ {' '.join(cmd)}")
803781
return subprocess.run(cmd, check=check, cwd=cwd)
804782

@@ -883,9 +861,7 @@ def main() -> None:
883861
),
884862
formatter_class=argparse.RawDescriptionHelpFormatter,
885863
)
886-
p.add_argument(
887-
"--store", default=DEFAULT_STORE, help="Store spec (default: %(default)s)"
888-
)
864+
p.add_argument("--store", default=DEFAULT_STORE, help="Store spec (default: %(default)s)")
889865
p.add_argument(
890866
"--git-ref",
891867
help="Git ref for version detection (e.g. v0.62.0). "
@@ -909,7 +885,8 @@ def main() -> None:
909885
"(hash-verified, skips unchanged files, errors on mismatches)",
910886
)
911887
p.add_argument(
912-
"--yes", "-y",
888+
"--yes",
889+
"-y",
913890
action="store_true",
914891
help="Skip confirmation prompt",
915892
)
@@ -931,9 +908,7 @@ def main() -> None:
931908
),
932909
formatter_class=argparse.RawDescriptionHelpFormatter,
933910
)
934-
p.add_argument(
935-
"--store", default=DEFAULT_STORE, help="Store spec (default: %(default)s)"
936-
)
911+
p.add_argument("--store", default=DEFAULT_STORE, help="Store spec (default: %(default)s)")
937912
p.add_argument(
938913
"--versions",
939914
help="Comma-separated versions to check (default: all)",
@@ -952,9 +927,7 @@ def main() -> None:
952927
),
953928
formatter_class=argparse.RawDescriptionHelpFormatter,
954929
)
955-
p.add_argument(
956-
"--store", default=DEFAULT_STORE, help="Store spec (default: %(default)s)"
957-
)
930+
p.add_argument("--store", default=DEFAULT_STORE, help="Store spec (default: %(default)s)")
958931
p.add_argument("version", nargs="?", help="Show manifest for this version")
959932

960933
# -- verify --
@@ -966,16 +939,10 @@ def main() -> None:
966939
"SHA-256 hash matches the manifest. Also checks that all files\n"
967940
"listed in manifests are present in the store."
968941
),
969-
epilog=(
970-
"examples:\n"
971-
" uv run compat.py verify\n"
972-
" uv run compat.py verify --store /tmp/store"
973-
),
942+
epilog=("examples:\n uv run compat.py verify\n uv run compat.py verify --store /tmp/store"),
974943
formatter_class=argparse.RawDescriptionHelpFormatter,
975944
)
976-
p.add_argument(
977-
"--store", default=DEFAULT_STORE, help="Store spec (default: %(default)s)"
978-
)
945+
p.add_argument("--store", default=DEFAULT_STORE, help="Store spec (default: %(default)s)")
979946

980947
# -- validate-manifest --
981948
p = sub.add_parser(
@@ -986,15 +953,11 @@ def main() -> None:
986953
"New fixtures are allowed; removals are errors."
987954
),
988955
epilog=(
989-
"examples:\n"
990-
" uv run compat.py validate-manifest\n"
991-
" uv run compat.py validate-manifest --store /tmp/store"
956+
"examples:\n uv run compat.py validate-manifest\n uv run compat.py validate-manifest --store /tmp/store"
992957
),
993958
formatter_class=argparse.RawDescriptionHelpFormatter,
994959
)
995-
p.add_argument(
996-
"--store", default=DEFAULT_STORE, help="Store spec (default: %(default)s)"
997-
)
960+
p.add_argument("--store", default=DEFAULT_STORE, help="Store spec (default: %(default)s)")
998961

999962
args = parser.parse_args()
1000963

‎vortex-test/compat-gen/src/adapter.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ use vortex_array::DynArray;
2222
use vortex_array::stream::ArrayStreamAdapter;
2323
use vortex_array::stream::ArrayStreamExt;
2424
use vortex_buffer::ByteBuffer;
25-
use vortex_error::{vortex_err, VortexResult};
25+
use vortex_error::VortexResult;
26+
use vortex_error::vortex_err;
2627
use vortex_session::VortexSession;
2728

2829
fn runtime() -> VortexResult<Runtime> {

0 commit comments

Comments
 (0)