Laszlo Gaal has posted comments on this change. ( http://gerrit.cloudera.org:8080/24839 )
Change subject: IMPALA-15146: Extend Impala Minicluster with an S3-compatible object store for testing vended credentials ...................................................................... Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/24839/5/testdata/bin/minicluster_lakekeeper_s3/setup.sh File testdata/bin/minicluster_lakekeeper_s3/setup.sh: http://gerrit.cloudera.org:8080/#/c/24839/5/testdata/bin/minicluster_lakekeeper_s3/setup.sh@110 PS5, Line 110: curl -f -s -X POST "${CATALOG_URL}/namespaces/ice_s3/tables" \ : -H "Authorization: Bearer $TOKEN" \ : -H "Content-Type: application/json" \ : --data "{ : \"name\": \"nation\", : \"location\": \"${TABLE_LOCATION}\", : \"schema\": { : \"type\": \"struct\", : \"schema-id\": 0, : \"fields\": [ : {\"id\": 1, \"name\": \"n_nationkey\", \"required\": false, \"type\": \"int\"}, : {\"id\": 2, \"name\": \"n_name\", \"required\": false, \"type\": \"string\"}, : {\"id\": 3, \"name\": \"n_regionkey\", \"required\": false, \"type\": \"int\"}, : {\"id\": 4, \"name\": \"n_comment\", \"required\": false, \"type\": \"string\"} : ] : }, : \"partition-spec\": {\"spec-id\": 0, \"fields\": []}, : \"write-order\": {\"order-id\": 0, \"fields\": []}, : \"properties\": {} : }" \ : -o /dev/null Although I'm sure this works, but it sure looks ugly: the amount of escaping the shell needs makes it hard to parse the structure visually. Since the data generator is Python, have you considered implementing the whole setup logic in Python? That may allow this logic and the file generator to get merged, and handling the replacement of the single TABLE_LOCATION parameter could also become simpler (and the data structure look nicer) in Python. I don't feel strongly about this, but 150 lines of heavily quoted and escaped shell code is not too pleasant to look at :) http://gerrit.cloudera.org:8080/#/c/24839/5/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.py File testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.py: http://gerrit.cloudera.org:8080/#/c/24839/5/testdata/cluster/node_templates/common/etc/hadoop/conf/core-site.xml.py@180 PS5, Line 180: # S3_ENDPOINT points at an S3-compatible service ( Should this be skipped if the primary FS is set to S3 to prevent interference with S3-based testing? Or would it be better to manage that exclusion by setting (or omitting) the env var S3_ENDPOINT itself in some other location? http://gerrit.cloudera.org:8080/#/c/24839/5/tests/custom_cluster/test_iceberg_credential_vending.py File tests/custom_cluster/test_iceberg_credential_vending.py: http://gerrit.cloudera.org:8080/#/c/24839/5/tests/custom_cluster/test_iceberg_credential_vending.py@61 PS5, Line 61: class TestIcebergCredentialVending(CustomClusterTestSuite): Should this be decorated with a @SkipIF S3 (or similar)? Also, it's not clear if this logic is conditional on S3_ENDPOINT being set; this script does not check it. -- To view, visit http://gerrit.cloudera.org:8080/24839 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I913b43300f8e0c7052b0b1fea609dc0ad50bce9b Gerrit-Change-Number: 24839 Gerrit-PatchSet: 5 Gerrit-Owner: Peter Rozsa <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Mon, 28 Sep 2026 22:47:54 +0000 Gerrit-HasComments: Yes
