mengw15 commented on code in PR #6860:
URL: https://github.com/apache/texera/pull/6860#discussion_r3780041727
##########
bin/local-dev/main.sh:
##########
@@ -707,7 +707,7 @@ export
STORAGE_LAKEFS_AUTH_API_SECRET="${STORAGE_LAKEFS_AUTH_API_SECRET:-random_
export
TEXERA_DASHBOARD_SERVICE_ENDPOINT="${TEXERA_DASHBOARD_SERVICE_ENDPOINT:-http://localhost:8080}";
export
WORKFLOW_COMPILING_SERVICE_ENDPOINT="${WORKFLOW_COMPILING_SERVICE_ENDPOINT:-http://localhost:9090}";
export
WORKFLOW_EXECUTION_SERVICE_ENDPOINT="${WORKFLOW_EXECUTION_SERVICE_ENDPOINT:-http://localhost:8085}";
-export
FILE_SERVICE_GET_PRESIGNED_URL_ENDPOINT="${FILE_SERVICE_GET_PRESIGNED_URL_ENDPOINT:-http://localhost:9092/api/dataset/presign-download}";
+export
FILE_SERVICE_GET_DATASET_PRESIGNED_URL_ENDPOINT="${FILE_SERVICE_GET_DATASET_PRESIGNED_URL_ENDPOINT:-http://localhost:9092/api/dataset/presign-download}";
Review Comment:
This setter moved to the new name, but `bin/single-node/.env:84` still
exports the old `FILE_SERVICE_GET_PRESIGNED_URL_ENDPOINT`. Grepping the PR
head, that file is the only remaining site of the old spelling — every consumer
(the Python document, `EnvironmentalVariable`, both k8s deployment templates,
and this file) now reads the new one, so the value set there is no longer read
by anything.
It fails silently rather than loudly: with the variable unset under its new
name, `dataset_file_document.py` falls back to its hardcoded
`http://localhost:9092/api/dataset/presign-download`, but single-node needs the
compose service name `http://file-service:9092/...`. From inside the UDF
container `localhost` is that container, so dataset reads from a UDF would stop
reaching file-service in the single-node deployment. Nothing catches it — no
test reads deployment env files and CI doesn't stand up single-node.
##########
amber/src/main/python/pytexera/storage/dataset_file_document.py:
##########
@@ -89,7 +89,7 @@ def __init__(self, file_path: str):
raise invalid_format
self.jwt_token = os.getenv("USER_JWT_TOKEN")
- self.presign_endpoint =
os.getenv("FILE_SERVICE_GET_PRESIGNED_URL_ENDPOINT")
+ self.presign_endpoint =
os.getenv("FILE_SERVICE_GET_DATASET_PRESIGNED_URL_ENDPOINT")
Review Comment:
`build / pyamber` is red on all three Python versions from this line and its
test counterparts: `ruff check` passes, but `ruff format --check` reports
`dataset_file_document.py` and `test_dataset_file_document.py` as needing
reformatting — the longer variable name pushes these past the line limit. A
single `ruff format src/main/python src/test/python` should clear all three
legs.
--
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]
