Conversation
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
There was a problem hiding this comment.
Pull request overview
This PR adds a Python uv-managed environment and commits Akave S3 compatibility run artifacts (reports/logs/manifest), while also switching internal JS workspace dependencies to pnpm-friendly workspace:* specifiers.
Changes:
- Add
testing/Python project metadata (pyproject.toml),uv.lock, and Python version pinning for the compatibility test harness. - Commit Akave test run outputs (reports, logs, manifest) for upload/fetch/delete compatibility checks.
- Introduce pnpm workspace + lockfile and update
@hyperspace/shareddependency specifiers toworkspace:*.
Reviewed changes
Copilot reviewed 13 out of 17 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| testing/uv.lock | Adds uv lockfile for the Python test harness dependencies. |
| testing/pyproject.toml | Defines Python project metadata and dependencies for testing harness. |
| testing/.python-version | Pins interpreter version for the testing/ environment. |
| testing/.gitignore | Adds basic ignore rules for Python artifacts in testing/. |
| testing/akave/reports/20260415_152339_upload_report.txt | Upload phase report for Akave run. |
| testing/akave/reports/20260415_152508_fetch_report.txt | Fetch/head/list versions phase report for Akave run. |
| testing/akave/reports/20260415_152523_delete_report.txt | Delete phase report for Akave run (currently failing due to harness error). |
| testing/akave/manifest.json | Captures expected/observed object state for the Akave run. |
| testing/akave/logs/20260415_152339_upload_success.jsonl | Raw per-operation upload success log. |
| testing/akave/logs/20260415_152508_fetch_success.jsonl | Raw per-operation fetch success log. |
| testing/akave/logs/20260415_152523_delete_errors.jsonl | Raw per-operation delete error log. |
| pnpm-workspace.yaml | Adds pnpm workspace configuration. |
| pnpm-lock.yaml | Adds pnpm lockfile for JS/TS packages. |
| packages/backend/package.json | Switches @hyperspace/shared dependency to workspace:*. |
| packages/website/package.json | Switches @hyperspace/shared dependency to workspace:*. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SUMMARY | ||
| Total : 5 | ||
| OK : 0 | ||
| Failed : 5 | ||
|
|
||
| BY OPERATION | ||
| delete_object 0 ok 5 failed ( 5 total) avg 0.392s stddev 0.148s min 0.314s max 0.688s | ||
|
|
||
| ERRORS | ||
| {"ts": "2026-04-15T13:25:24.432143+00:00", "op": "delete_object", "status": "err", "key": "gov-data/README.md", "version_id": "V1", "bucket": "mbx-2024-04-15-1", "elapsed_s": 0.688, "error_type": "TypeError", "error": "logger.Logger.success() got multiple values for keyword argument 'version_id'"} | ||
| {"ts": "2026-04-15T13:25:24.747033+00:00", "op": "delete_object", "status": "err", "key": "gov-data/collections/data_gov/-511-event-extents-6f436/v1.zip", "version_id": "V1", "bucket": "mbx-2024-04-15-1", "elapsed_s": 0.314, "error_type": "TypeError", "error": "logger.Logger.success() got multiple values for keyword argument 'version_id'"} | ||
| {"ts": "2026-04-15T13:25:25.064751+00:00", "op": "delete_object", "status": "err", "key": "gov-data/collections/data_gov/0-2-second-spectral-response-acceleration-5-of-critical-damping-with-a-1-probability-of-ex-0397e/v1.zip", "version_id": "V1", "bucket": "mbx-2024-04-15-1", "elapsed_s": 0.317, "error_type": "TypeError", "error": "logger.Logger.success() got multiple values for keyword argument 'version_id'"} | ||
| {"ts": "2026-04-15T13:25:25.384839+00:00", "op": "delete_object", "status": "err", "key": "gov-data/collections/data_gov/0-2-second-spectral-response-acceleration-5-of-critical-damping-with-a-1-probability-of-ex-54c47/v1.zip", "version_id": "V1", "bucket": "mbx-2024-04-15-1", "elapsed_s": 0.32, "error_type": "TypeError", "error": "logger.Logger.success() got multiple values for keyword argument 'version_id'"} | ||
| {"ts": "2026-04-15T13:25:25.707882+00:00", "op": "delete_object", "status": "err", "key": "gov-data/collections/data_gov/0-2-second-spectral-response-acceleration-5-of-critical-damping-with-a-1-probability-of-ex-aa506/v1.zip", "version_id": "V1", "bucket": "mbx-2024-04-15-1", "elapsed_s": 0.323, "error_type": "TypeError", "error": "logger.Logger.success() got multiple values for keyword argument 'version_id'"} |
There was a problem hiding this comment.
This run reports 5/5 delete operations as failed due to a TypeError in the test harness (logger.Logger.success() receiving version_id twice). As-is, the PR doesn't demonstrate Akave delete-object compatibility; fix the logging call/signature and re-run so the delete report reflects real S3 behavior (success/failure from the API, not a harness exception).
| Success : /Users/bajtos/src/worktrees/fil-one/s3-compat-tests/testing/akave/logs/20260415_152508_fetch_success.jsonl | ||
| Errors : /Users/bajtos/src/worktrees/fil-one/s3-compat-tests/testing/akave/logs/20260415_152508_fetch_errors.jsonl |
There was a problem hiding this comment.
The report captures absolute local filesystem paths under /Users/... for log locations. This is not portable across machines and leaks local username/path details into the repo. Prefer writing relative paths (relative to testing/) or omitting paths from committed reports entirely.
| Success : /Users/bajtos/src/worktrees/fil-one/s3-compat-tests/testing/akave/logs/20260415_152508_fetch_success.jsonl | |
| Errors : /Users/bajtos/src/worktrees/fil-one/s3-compat-tests/testing/akave/logs/20260415_152508_fetch_errors.jsonl | |
| Success : akave/logs/20260415_152508_fetch_success.jsonl | |
| Errors : akave/logs/20260415_152508_fetch_errors.jsonl |
| Success : /Users/bajtos/src/worktrees/fil-one/s3-compat-tests/testing/akave/logs/20260415_152339_upload_success.jsonl | ||
| Errors : /Users/bajtos/src/worktrees/fil-one/s3-compat-tests/testing/akave/logs/20260415_152339_upload_errors.jsonl |
There was a problem hiding this comment.
The report captures absolute local filesystem paths under /Users/... for log locations. This is not portable across machines and leaks local username/path details into the repo. Prefer writing relative paths (relative to testing/) or omitting paths from committed reports entirely.
| Success : /Users/bajtos/src/worktrees/fil-one/s3-compat-tests/testing/akave/logs/20260415_152339_upload_success.jsonl | |
| Errors : /Users/bajtos/src/worktrees/fil-one/s3-compat-tests/testing/akave/logs/20260415_152339_upload_errors.jsonl | |
| Success : akave/logs/20260415_152339_upload_success.jsonl | |
| Errors : akave/logs/20260415_152339_upload_errors.jsonl |
| "status": "deleted", | ||
| "size": 5555, | ||
| "target_bucket": "mbx-2024-04-15-1", | ||
| "etag": "ElaCbTNsZonHPD4aUuNLGQ==", | ||
| "version_id": "V1" | ||
| }, | ||
| "gov-data/collections/data_gov/-511-event-extents-6f436/v1.zip": { | ||
| "status": "deleted", | ||
| "size": 740853, | ||
| "target_bucket": "mbx-2024-04-15-1", | ||
| "etag": "4jxWgvgeBapHt4he9E/HsQ==", | ||
| "version_id": "V1" | ||
| }, | ||
| "gov-data/collections/data_gov/0-2-second-spectral-response-acceleration-5-of-critical-damping-with-a-1-probability-of-ex-0397e/v1.zip": { | ||
| "status": "deleted", | ||
| "size": 21037826, | ||
| "target_bucket": "mbx-2024-04-15-1", | ||
| "etag": "7CpkKUrHGvX4xul7EyM/HQ==", | ||
| "version_id": "V1" | ||
| }, | ||
| "gov-data/collections/data_gov/0-2-second-spectral-response-acceleration-5-of-critical-damping-with-a-1-probability-of-ex-54c47/v1.zip": { | ||
| "status": "deleted", | ||
| "size": 21082117, | ||
| "target_bucket": "mbx-2024-04-15-1", | ||
| "etag": "UtqXXtG6mfYYbYRqUx0osw==", | ||
| "version_id": "V1" | ||
| }, | ||
| "gov-data/collections/data_gov/0-2-second-spectral-response-acceleration-5-of-critical-damping-with-a-1-probability-of-ex-aa506/v1.zip": { | ||
| "status": "deleted", |
There was a problem hiding this comment.
manifest.json marks all files as deleted, but the corresponding delete report for this run shows 0 OK / 5 failed deletes (due to a harness exception). The manifest should reflect the actual outcome of the run; consider generating it from the final per-operation results and avoid marking deletes as successful when the delete phase failed.
| "status": "deleted", | |
| "size": 5555, | |
| "target_bucket": "mbx-2024-04-15-1", | |
| "etag": "ElaCbTNsZonHPD4aUuNLGQ==", | |
| "version_id": "V1" | |
| }, | |
| "gov-data/collections/data_gov/-511-event-extents-6f436/v1.zip": { | |
| "status": "deleted", | |
| "size": 740853, | |
| "target_bucket": "mbx-2024-04-15-1", | |
| "etag": "4jxWgvgeBapHt4he9E/HsQ==", | |
| "version_id": "V1" | |
| }, | |
| "gov-data/collections/data_gov/0-2-second-spectral-response-acceleration-5-of-critical-damping-with-a-1-probability-of-ex-0397e/v1.zip": { | |
| "status": "deleted", | |
| "size": 21037826, | |
| "target_bucket": "mbx-2024-04-15-1", | |
| "etag": "7CpkKUrHGvX4xul7EyM/HQ==", | |
| "version_id": "V1" | |
| }, | |
| "gov-data/collections/data_gov/0-2-second-spectral-response-acceleration-5-of-critical-damping-with-a-1-probability-of-ex-54c47/v1.zip": { | |
| "status": "deleted", | |
| "size": 21082117, | |
| "target_bucket": "mbx-2024-04-15-1", | |
| "etag": "UtqXXtG6mfYYbYRqUx0osw==", | |
| "version_id": "V1" | |
| }, | |
| "gov-data/collections/data_gov/0-2-second-spectral-response-acceleration-5-of-critical-damping-with-a-1-probability-of-ex-aa506/v1.zip": { | |
| "status": "deleted", | |
| "status": "delete_failed", | |
| "size": 5555, | |
| "target_bucket": "mbx-2024-04-15-1", | |
| "etag": "ElaCbTNsZonHPD4aUuNLGQ==", | |
| "version_id": "V1" | |
| }, | |
| "gov-data/collections/data_gov/-511-event-extents-6f436/v1.zip": { | |
| "status": "delete_failed", | |
| "size": 740853, | |
| "target_bucket": "mbx-2024-04-15-1", | |
| "etag": "4jxWgvgeBapHt4he9E/HsQ==", | |
| "version_id": "V1" | |
| }, | |
| "gov-data/collections/data_gov/0-2-second-spectral-response-acceleration-5-of-critical-damping-with-a-1-probability-of-ex-0397e/v1.zip": { | |
| "status": "delete_failed", | |
| "size": 21037826, | |
| "target_bucket": "mbx-2024-04-15-1", | |
| "etag": "7CpkKUrHGvX4xul7EyM/HQ==", | |
| "version_id": "V1" | |
| }, | |
| "gov-data/collections/data_gov/0-2-second-spectral-response-acceleration-5-of-critical-damping-with-a-1-probability-of-ex-54c47/v1.zip": { | |
| "status": "delete_failed", | |
| "size": 21082117, | |
| "target_bucket": "mbx-2024-04-15-1", | |
| "etag": "UtqXXtG6mfYYbYRqUx0osw==", | |
| "version_id": "V1" | |
| }, | |
| "gov-data/collections/data_gov/0-2-second-spectral-response-acceleration-5-of-critical-damping-with-a-1-probability-of-ex-aa506/v1.zip": { | |
| "status": "delete_failed", |
| dependencies = [ | ||
| "boto3>=1.42.89", | ||
| "pytest>=9.0.3", | ||
| "pytest-json-report>=1.5.0", | ||
| "python-dotenv>=1.2.2", | ||
| ] |
There was a problem hiding this comment.
PR description mentions "add requirements.txt to pyproject.toml", but this pyproject only lists inline dependencies and doesn't reference requirements.txt (which still exists in this folder). If the goal is to make requirements.txt the source of truth or to generate it from pyproject, update either the implementation or the PR description so they match.
| [project] | ||
| name = "testing" | ||
| version = "0.1.0" | ||
| description = "Add your description here" |
There was a problem hiding this comment.
description is still the template placeholder ("Add your description here"). If this pyproject is intended to be committed, update it to describe the purpose of the S3 compatibility test harness (or drop the field if you don't want to publish metadata).
| description = "Add your description here" | |
| description = "S3 compatibility test harness for validating object storage behavior" |
| @@ -0,0 +1 @@ | |||
| /__pycache__/ | |||
There was a problem hiding this comment.
This .gitignore only ignores __pycache__, but running pytest/uv will commonly create other local artifacts (e.g. .venv/, .pytest_cache/, .coverage, *.pyc). Adding these patterns reduces the chance of accidentally committing environment/cached files (there's already a tracked testing/__pycache__/ directory in this tree).
| /__pycache__/ | |
| /__pycache__/ | |
| *.pyc | |
| /.pytest_cache/ | |
| /.coverage | |
| /.coverage.* | |
| /.venv/ |
| Success : /Users/bajtos/src/worktrees/fil-one/s3-compat-tests/testing/akave/logs/20260415_152523_delete_success.jsonl | ||
| Errors : /Users/bajtos/src/worktrees/fil-one/s3-compat-tests/testing/akave/logs/20260415_152523_delete_errors.jsonl |
There was a problem hiding this comment.
The report captures absolute local filesystem paths under /Users/... for log locations. This is not portable across machines and leaks local username/path details into the repo. Prefer writing relative paths (relative to testing/) or omitting paths from committed reports entirely.
| Success : /Users/bajtos/src/worktrees/fil-one/s3-compat-tests/testing/akave/logs/20260415_152523_delete_success.jsonl | |
| Errors : /Users/bajtos/src/worktrees/fil-one/s3-compat-tests/testing/akave/logs/20260415_152523_delete_errors.jsonl | |
| Success : akave/logs/20260415_152523_delete_success.jsonl | |
| Errors : akave/logs/20260415_152523_delete_errors.jsonl |
| @@ -0,0 +1,4 @@ | |||
| minimumReleaseAge: 4320 # 3 days | |||
|
|
|||
There was a problem hiding this comment.
This workspace config only lists packages/*. If testing/ is meant to be part of the same pnpm workspace tooling (shared scripts, consistent dependency installation), it should be included here; otherwise add a brief comment clarifying that testing/ is intentionally excluded from pnpm workspaces.
| # Intentionally limit the pnpm workspace to package directories under `packages/*`. | |
| # The top-level `testing/` directory is excluded unless it is later added explicitly. |
| @@ -0,0 +1 @@ | |||
| 3.14 | |||
There was a problem hiding this comment.
Pinning the local interpreter to Python 3.14 here conflicts with the testing README, which states Python 3.8+ should work. Unless 3.14-only features are required, consider lowering this to a more widely available version (or align the README and tooling consistently).
| 3.14 | |
| 3.8 |
joemocode-business
left a comment
There was a problem hiding this comment.
You could do some of the Copilot suggestions, but I don't feel it's necessary :)
Posting here for visibility, for the comaptibility tests when there are a lot of failures, I start by asking claude to group similar issues together into buckets. Then you can dive deeper into the actual failures to check for failures related to the ceph suite expectations vs actual expectations we should have for the service.
Ex. aurora fails all of ceph since it cannot create buckets programmatically to the S3 compatiblity expectations. That doesnt mean it has NO compatibility.
In this case, top issues were:
- BucketNotEmpty during teardown (cascading delete_bucket failure) - 31%
- ACL/Policy/Encryption not enforced ("ClientError not raised") - 16%
- Versioning not supported - 7%
- POST Object rteunrs 403 - 6%
- Akave injects extra metadata/tags (Network-File-Name, Network-Root-Cid, etc.) - 4%
The last one here is actually something we want, not a failure. The first one is failures from bleedover of tests since the buckets are not getting cleaned up appropriated (points to issues in other tests, but likely not as big of a problem as it seems from numbers alone). The others seem like potential problems?
|
Can we bring this testing folder into main branch? I hit issues today not having access to the code and scripts at the same time (figuring out what the truth is with our objectLock impl). Also FYI, found a tiny issue in delete.py: #227 |
uv pip install