Fokko commented on code in PR #2482:
URL: https://github.com/apache/iceberg-python/pull/2482#discussion_r2368867949
##########
tests/integration/test_catalog.py:
##########
@@ -74,6 +76,20 @@ def rest_catalog() -> Generator[Catalog, None, None]:
clean_up(test_catalog)
[email protected](scope="function")
+def rest_catalog_env() -> Generator[Catalog, None, None]:
+ if properties_json := os.environ.get("PYICEBERG_REST_CATALOG_PROPERTIES"):
+ properties = json.loads(properties_json)
+ test_catalog = RestCatalog(
+ "rest_env",
+ **properties,
+ )
Review Comment:
Looking at it a bit more, there are two ways of setting the properties now.
For example, if you had a `~/.pyiceberg.yaml` like:
```yaml
catalog:
rest_env:
uri: http://rest-catalog/ws/
credential: t-1234:secret
```
or, you would set more environment variables:
```sh
export PYICEBERG_CATALOG__REST_ENV__URI=thrift://localhost:9083
export PYICEBERG_CATALOG__REST_ENV__ACCESS_KEY_ID=username
export PYICEBERG_CATALOG__REST_ENV__SECRET_ACCESS_KEY=password
```
Then they would inluence the behavior of the test catalog. Maybe we can do
it like:
```suggestion
if test_catalog_name := os.environ.get("PYICEBERG_TEST_CATALOG"):
test_catalog = RestCatalog(test_catalog_name)
```
This way, we leverage the current way of passing in configuration.
Nit on the side, the variable names suggest REST, but there is no need to
constrain this to a REST catalog.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
