ShawnGarello opened a new pull request, #8978: URL: https://github.com/apache/texera/pull/8978
### What changes were proposed in this PR? `FileScanSourceOpExecSpec`'s test "create LargeBinary when reading file with LARGE_BINARY type" streams a real object to S3 whenever an endpoint is reachable (`s3://texera-large-binaries/objects/8888/<uuid>`), and nothing deleted it. Since #7800 made the test actually reach S3, every local run with the dev stack up (`bin/local-dev.sh up` publishes RustFS on :9000 with the credentials `storage.conf` uses) has added one more object to the developer's persistent `rustfs_data` volume. CI never noticed: the `amber` job has no S3, so the test cancels at its probe, and the `amber-integration` jobs start RustFS but only run `IntegrationTest`-tagged tests (`AMBER_TEST_FILTER=integration-only`), which this one isn't. This test-only change, in that one file: 1. Calls `LargeBinaryManager.deleteByExecution(testExecutionId)` in the test's existing outer `finally`, so the cleanup also runs when the scan throws. `deleteByExecution` logs failures instead of throwing, so it can't hide the test's own failure. 2. Asserts at the end of the test that nothing is left under `objects/<testExecutionId>/` (`S3StorageClient.directoryExists`). Without the cleanup, the test fails. 3. Changes `testExecutionId` from `8888L` to `-8670L`. The new cleanup is a prefix delete against whatever S3 the config points at, which outside CI is a developer's own. Real execution ids are positive (`workflow_executions.eid` is `SERIAL`), so `objects/-8670/` can never hold a real execution's binaries, while `8888` could be a real execution on a long-lived dev instance. `directoryExists`/`deleteDirectory` add the trailing `/`, so `-8670` doesn't match `-86700`. I also considered a random negative id per run, which would also keep two checkouts that share one RustFS apart, but chose a fixed one so that any leftover is easy to find if the cleanup ever fails. (`baseUriForExecution`'s doc asks for a persisted EID; its reason is avoiding collisions with real executions, which a negative id can't have. Happy to change this if you'd prefer something else.) Not covered: objects that earlier runs already left under `objects/8888/` are not removed by this change. They can be deleted by hand, e.g. `aws s3 rm s3://texera-large-binaries/objects/8888/ --recursive --endpoint-url http://localhost:9000`. Possible follow-ups, not in this PR: - Tag this test `IntegrationTest`, so the `amber-integration` job (which has RustFS) runs it. - Give WorkflowOperator's test scope the `S3StorageTestBase` testcontainers RustFS, so the test is hermetic and runs in CI. ### Any related issues, documentation, discussions? Closes #8670. Follow-up to #7800. Backport: not needed. On `release/v1.2` and `release/v1.3` this spec has no `8888` and no `setCurrentBaseUri`, so the leak isn't there. @mengw15 @xuang7, if backport labels get auto-applied, please drop them. ### How was this PR tested? Locally against Postgres and RustFS (`rustfs/rustfs:1.0.0-rc.6` on localhost:9000, `storage.conf` defaults), with `sbt "WorkflowOperator/testOnly *FileScanSourceOpExecSpec"`: 1. Red: with only the new assertion (id `8888`, no cleanup), the test fails with `directoryExists(...) was true objects/8888/`, and one 26-byte object is left behind. 2. Green: with the full change, on a fresh RustFS, two runs both pass (7/7), and the bucket is empty after each. 3. With the cleanup line commented out, the test fails with `... was true objects/-8670/`, and the object sits at `objects/-8670/<uuid>`. With the line restored, the next run passes and removes it. An object placed under `objects/-86700/` survives. 4. Without S3: `succeeded 6, failed 0, canceled 1` with the existing skip message, the same as the CI `amber` job. 5. `sbt "WorkflowOperator/test"` with S3 up: 2702 succeeded, 0 failed, 12 canceled, 2 pending. 6. `sbt scalafmtCheckAll "scalafixAll --check"`: clean. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 5.5) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
