visit2rahul commented on PR #4422:
URL: https://github.com/apache/polaris/pull/4422#issuecomment-4456152177
CI was red on `cc58e79a6` for two unrelated reasons — both now fixed in
`04fe8f939`:
**1. Spotless (`Gradle Build Checks`)**
`spotlessJavaCheck` flagged a javadoc wrap in
`BaseLocationValidationTest.java`. Moved the trailing word so the line meets
google-java-format's preferred width. Verified locally:
```
./gradlew :polaris-runtime-service:spotlessJavaCheck
BUILD SUCCESSFUL
```
**2. Regression tests (`Regression Tests (minio)` and `(rustfs)`)**
`t_cli/src/test_cli.py::test_update_catalog` was exercising exactly the
behavior this PR forbids — it created a catalog with
`allowedLocations=[s3://fake-location-{SALT}]` and then updated
`--default-base-location` to `s3://new-location-{SALT}` with no corresponding
storage-config update. The server now (correctly) rejects that with
`BadRequestException`.
Two changes:
- Updated the existing call to pass `--allowed-location` alongside
`--default-base-location` so the storage config is updated atomically. Per the
validation in `PolarisAdminService.java#L953-957`, the check is skipped when
`updateRequest.getStorageConfigInfo() != null`, which is the correct semantics
(the catalog can't end up inconsistent if the storage config is being updated
in the same call).
- Added `test_update_catalog_base_location_validation` to cover the new
behavior explicitly: (a) base-only change to an outside location is rejected
with the expected message, (b) base is unchanged after rejection, (c)
simultaneous base + allowed-location update is permitted.
@jbonofre — let me know if you'd prefer the existing test left alone and
only the negative test added, or any other shape for the coverage.
--
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]
