Copilot commented on code in PR #4409:
URL: https://github.com/apache/texera/pull/4409#discussion_r3104532872
##########
amber/src/main/python/core/storage/iceberg/iceberg_utils.py:
##########
@@ -148,7 +148,7 @@ def create_postgres_catalog(
catalog_name,
**{
"uri":
f"postgresql+pg8000://{username}:{password}@{uri_without_scheme}",
- "warehouse": f"file://{warehouse_path}",
+ "warehouse": warehouse_path,
},
Review Comment:
On Windows, the Java/Scala Postgres JdbcCatalog initializes the warehouse
using `warehouse.toString.replace(":", "")` (see
`common/workflow-core/src/main/scala/org/apache/texera/amber/util/IcebergUtil.scala:141-145`).
Python now passes `warehouse_path` through unmodified, which will likely
reintroduce a Python-vs-Scala mismatch for drive-letter paths (e.g., `C:/...`
vs `C/...`) and break cross-runtime table reads again. Consider applying the
same normalization here (or ensuring the value passed into the Python worker is
already normalized) so both runtimes persist the exact same warehouse string.
##########
amber/src/main/python/core/storage/iceberg/iceberg_utils.py:
##########
@@ -148,7 +148,7 @@ def create_postgres_catalog(
catalog_name,
**{
"uri":
f"postgresql+pg8000://{username}:{password}@{uri_without_scheme}",
- "warehouse": f"file://{warehouse_path}",
+ "warehouse": warehouse_path,
},
Review Comment:
This change is fixing a cross-runtime compatibility issue by altering the
persisted `warehouse` value. There are pytest tests for other helpers in this
module (e.g., `core/storage/iceberg/test_iceberg_utils_large_binary.py`), but
nothing guarding `create_postgres_catalog` behavior. Adding a unit test that
mocks/patches `SqlCatalog` and asserts the `warehouse` argument is passed
without the `file://` prefix (and with any required normalization) would help
prevent regressions.
--
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]
