You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "GayathriSrividya (via GitHub)" <gi...@apache.org> on 2026/05/28 03:20:03 UTC
[PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
GayathriSrividya opened a new pull request, #3423:
URL: https://github.com/apache/iceberg-python/pull/3423
## Summary
Fixes #3422.
`RestCatalog._create_session()` expected `auth` to be a `dict`, but values loaded from environment variables always arrive as strings. This caused auth initialization to fail silently or with a confusing `AttributeError` for all pluggable auth types (`basic`, `oauth2`, `google`, `entra`, `custom`) when configured via `PYICEBERG_CATALOG__<NAME>__AUTH`.
## Fixes
### 1. JSON string decode (primary fix)
If the `auth` property value is a `str`, JSON-parse it before processing. This unblocks:
```bash
export PYICEBERG_CATALOG__REST__AUTH='{"type":"oauth2","oauth2":{"client_id":"id","client_secret":"secret","token_url":"https://auth.example/token"}}'
```
### 2. Flat env-var property support (alternative fix)
Supports the canonical PyIceberg env-var style — no JSON blob required, no quoting/escaping issues:
```bash
export PYICEBERG_CATALOG__REST__AUTH__TYPE=oauth2
export PYICEBERG_CATALOG__REST__AUTH__OAUTH2__CLIENT_ID=id
export PYICEBERG_CATALOG__REST__AUTH__OAUTH2__CLIENT_SECRET=secret
export PYICEBERG_CATALOG__REST__AUTH__OAUTH2__TOKEN_URL=https://auth.example/token
```
The env-var parser maps these to flat properties (`auth.type`, `auth.oauth2.client-id`, etc.). `_create_session` now checks for `auth.type` when the `auth` dict is absent, builds the config dict from the flat `auth.*` properties, and converts kebab-case keys to snake_case to match `AuthManager` constructor parameters.
## Tests
Five new regression tests added to `tests/catalog/test_rest.py`:
| Test | What it covers |
|---|---|
| `test_rest_catalog_with_basic_auth_as_json_string` | JSON string → Basic auth |
| `test_rest_catalog_with_oauth2_auth_as_json_string` | JSON string → OAuth2 auth |
| `test_rest_catalog_with_invalid_json_auth_string` | Invalid JSON → descriptive `ValueError` |
| `test_rest_catalog_with_basic_auth_flat_properties` | Flat `auth.*` props → Basic auth |
| `test_rest_catalog_with_oauth2_auth_flat_properties` | Flat `auth.*` props → OAuth2 auth (kebab→snake conversion) |
All 13 `test_rest_catalog_with_*` tests pass; the 4 pre-existing failures (`test_token_200`, `test_config_200`, `test_auth_header`, etc.) are unrelated to this change and also fail on `main`.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "GayathriSrividya (via GitHub)" <gi...@apache.org>.
GayathriSrividya commented on PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#issuecomment-4582433613
Thanks! I ran `make lint` locally and re-pushed the changes. CI is green now — please take another look when you have a chance.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "abnobdoss (via GitHub)" <gi...@apache.org>.
abnobdoss commented on code in PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#discussion_r3331154116
##########
tests/catalog/test_rest.py:
##########
@@ -3167,3 +3167,135 @@ def test_load_table_without_storage_credentials(
)
assert actual.metadata.model_dump() == expected.metadata.model_dump()
assert actual == expected
+
+
+# Tests for issue #3422: REST catalog auth cannot be configured via environment
+# variables unless auth JSON strings are decoded.
+
+
+def test_rest_catalog_with_basic_auth_as_json_string(rest_mock: Mocker) -> None:
Review Comment:
Thanks for addressing the comments! I think this change might not have gone through.
##########
tests/catalog/test_rest.py:
##########
@@ -3167,3 +3167,135 @@ def test_load_table_without_storage_credentials(
)
assert actual.metadata.model_dump() == expected.metadata.model_dump()
assert actual == expected
+
+
+# Tests for issue #3422: REST catalog auth cannot be configured via environment
+# variables unless auth JSON strings are decoded.
+
+
+def test_rest_catalog_with_basic_auth_as_json_string(rest_mock: Mocker) -> None:
Review Comment:
Thanks for addressing the comments! I think the testing change might not have gone through.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#issuecomment-5975272483
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#issuecomment-6115687323
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "rambleraptor (via GitHub)" <gi...@apache.org>.
rambleraptor commented on PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#issuecomment-4580058187
Thanks for writing this up! I've run into similar issues before (doing something very non-standard...) and I think this will be a huge help!
Can you run `make lint` and re-push?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "rambleraptor (via GitHub)" <gi...@apache.org>.
rambleraptor commented on PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#issuecomment-4961433585
@GayathriSrividya are you able to change this to just support flattened properties? I'd love to get this in before we release 0.12
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #3423: fix: support env-var auth config for REST catalog (JSON string decode + flat properties)
URL: https://github.com/apache/iceberg-python/pull/3423
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "GayathriSrividya (via GitHub)" <gi...@apache.org>.
GayathriSrividya commented on PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#issuecomment-4966058785
> @GayathriSrividya are you able to change this to just support flattened properties? I'd love to get this in before we release 0.12
Done, this now supports flattened auth properties only. I removed the JSON auth env path and updated tests to cover the flattened scenarios (including typed flat auth cases). Lint and focused REST auth tests are passing. Pushed in 532d0979.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "GayathriSrividya (via GitHub)" <gi...@apache.org>.
GayathriSrividya commented on code in PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#discussion_r3351932925
##########
pyiceberg/catalog/rest/__init__.py:
##########
@@ -435,7 +436,32 @@ def _create_session(self) -> Session:
elif ssl_client_cert := ssl_client.get(CERT):
session.cert = ssl_client_cert
- if auth_config := self.properties.get(AUTH):
+ raw_auth = self.properties.get(AUTH)
+ if isinstance(raw_auth, str):
+ try:
+ auth_config: dict[str, Any] | None = json.loads(raw_auth)
+ except json.JSONDecodeError as e:
+ raise ValueError("Failed to parse auth configuration as JSON") from e
+ elif raw_auth is not None:
+ auth_config = raw_auth
+ elif auth_type := self.properties.get(f"{AUTH}.type"):
+ type_prefix = f"{AUTH}.{auth_type}."
+ auth_config = {
Review Comment:
Thanks for the suggestion — agreed. I added explicit typed auth config structures and split parsing/validation into typed helper functions so this path is no longer handled as an untyped dict throughout.
The behavior is unchanged, and existing JSON-string + flat env-var auth tests still pass after the refactor.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "GayathriSrividya (via GitHub)" <gi...@apache.org>.
GayathriSrividya commented on code in PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#discussion_r3324275823
##########
pyiceberg/catalog/rest/__init__.py:
##########
Review Comment:
This suggestion is now implemented. The code initializes `auth_type = None` before the logic, and all custom validation and AuthManager initialization is gated on `if auth_type:`. This removes repetition and makes the flow clearer, as you recommended. Thank you!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "koenichiwa (via GitHub)" <gi...@apache.org>.
koenichiwa commented on PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#issuecomment-5522658045
What is the state of this PR? I'm vurrently trying to figure out how to inject client_id and client_secret on the fly
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#issuecomment-4759705245
Thank you for the PR. Im not sure if its a good idea to accept both JSON string and flattened properties. I think we should just pick one. My vote is the flattened properties, we dont really have other patterns of accepting JSON string as config.
Accepting both makes documentation confusing. and JSON string might contain other arbitrary key/value pairs
I would also like to keep changes to `pyiceberg/catalog/rest/__init__.py` to a minimum, seems like there are a lot of helper functions that might not be necessary. Hopefully dropping JSON string support will get rid of a lot of the changes
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "GayathriSrividya (via GitHub)" <gi...@apache.org>.
GayathriSrividya commented on PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#issuecomment-4589543108
> Thanks for the updates! I took another look and may be missing it, but I still only see the basic/simple OAuth2 test cases covered for flat `auth.*` env vars.
>
> Could we also cover the typed flat-auth cases, like OAuth2 numeric fields and Google/Entra scopes?
Thanks, I added coverage for the typed flat auth.* cases you called out. The flat env-var path now coerces typed auth-manager kwargs before construction, and the tests now cover OAuth2 numeric fields (refresh_margin, expires_in) plus Google and Entra scopes as list[str]. I also reran the focused REST auth tests and make lint.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "GayathriSrividya (via GitHub)" <gi...@apache.org>.
GayathriSrividya commented on code in PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#discussion_r3324285539
##########
pyiceberg/catalog/rest/__init__.py:
##########
@@ -435,7 +436,16 @@ def _create_session(self) -> Session:
elif ssl_client_cert := ssl_client.get(CERT):
session.cert = ssl_client_cert
- if auth_config := self.properties.get(AUTH):
+ if raw_auth := self.properties.get(AUTH):
Review Comment:
This suggestion is implemented. The code now sets `source_env_var = f"PYICEBERG_CATALOG__{self.name.upper()}__AUTH"` when loading from the dictionary env var, and includes this in error messages for clearer context. This makes custom auth type errors more descriptive, as you recommended. Thank you!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "abnobdoss (via GitHub)" <gi...@apache.org>.
abnobdoss commented on code in PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#discussion_r3321490725
##########
pyiceberg/catalog/rest/__init__.py:
##########
Review Comment:
Since these 6 lines are repeated would it make sense to init auth_type to None before line 439 and then use auth_type being set as the gate to run these 6 lines (the custom validation + the auth manager initialization)?
##########
pyiceberg/catalog/rest/__init__.py:
##########
@@ -435,7 +436,16 @@ def _create_session(self) -> Session:
elif ssl_client_cert := ssl_client.get(CERT):
session.cert = ssl_client_cert
- if auth_config := self.properties.get(AUTH):
+ if raw_auth := self.properties.get(AUTH):
+ # When auth is configured via an environment variable (e.g. PYICEBERG_CATALOG__<NAME>__AUTH),
+ # the value arrives as a JSON string rather than a dict. Decode it before processing.
+ if isinstance(raw_auth, str):
+ try:
+ auth_config: dict[str, Any] = json.loads(raw_auth)
+ except json.JSONDecodeError as e:
+ raise ValueError(f"Failed to parse auth configuration as JSON: {raw_auth!r}") from e
+ else:
+ auth_config = raw_auth
auth_type = auth_config.get("type")
Review Comment:
It's possible that a valid JSON was passed but that it's not a dictionary. Should we add a guard here that validates a dictionary was passed?
##########
pyiceberg/catalog/rest/__init__.py:
##########
@@ -435,7 +436,16 @@ def _create_session(self) -> Session:
elif ssl_client_cert := ssl_client.get(CERT):
session.cert = ssl_client_cert
- if auth_config := self.properties.get(AUTH):
+ if raw_auth := self.properties.get(AUTH):
Review Comment:
if we identify which env var we're using to load the catalog (e.g. here it's the dictionary env var) we can make the custom auth type errors later on a bit more clear.
```suggestion
if raw_auth := self.properties.get(AUTH):
source_env_var = f"PYICEBERG_CATALOG__{self.name.upper()}__AUTH"
```
##########
tests/catalog/test_rest.py:
##########
@@ -3167,3 +3167,135 @@ def test_load_table_without_storage_credentials(
)
assert actual.metadata.model_dump() == expected.metadata.model_dump()
assert actual == expected
+
+
+# Tests for issue #3422: REST catalog auth cannot be configured via environment
+# variables unless auth JSON strings are decoded.
+
+
+def test_rest_catalog_with_basic_auth_as_json_string(rest_mock: Mocker) -> None:
Review Comment:
Would it make sense to combine these two tests via parameterization so we can extend it and do the remaining auth types and a few edge cases? In particular google and entra accepts scopes as list[str] and oauth2 accepts refresh_margin + expires_in as ints - do those get parsed downstream or do those face similar parsing issues with json string?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#issuecomment-5518469743
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that's incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "GayathriSrividya (via GitHub)" <gi...@apache.org>.
GayathriSrividya commented on PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#issuecomment-4966057241
> Thank you for the PR. Im not sure if its a good idea to accept both JSON string and flattened properties. I think we should just pick one. My vote is the flattened properties, we dont really have other patterns of accepting JSON string as config.
>
> Accepting both makes documentation confusing. and JSON string might contain other arbitrary key/value pairs
>
> I would also like to keep changes to `pyiceberg/catalog/rest/__init__.py` to a minimum, seems like there are a lot of helper functions that might not be necessary. Hopefully dropping JSON string support will get rid of a lot of the changes. If we truly need to define helper methods, it might be better to add them to `pyiceberg/catalog/rest/auth.py` instead.
Thanks for the feedback, I’ve updated this to flattened properties only and removed support for parsing auth as a JSON string. I also kept the change surface minimal in rest init by using the existing auth config flow and removing the JSON auth path/tests. Pushed in 532d0979.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "GayathriSrividya (via GitHub)" <gi...@apache.org>.
GayathriSrividya commented on code in PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#discussion_r3324270830
##########
pyiceberg/catalog/rest/__init__.py:
##########
@@ -435,7 +436,16 @@ def _create_session(self) -> Session:
elif ssl_client_cert := ssl_client.get(CERT):
session.cert = ssl_client_cert
- if auth_config := self.properties.get(AUTH):
+ if raw_auth := self.properties.get(AUTH):
+ # When auth is configured via an environment variable (e.g. PYICEBERG_CATALOG__<NAME>__AUTH),
+ # the value arrives as a JSON string rather than a dict. Decode it before processing.
+ if isinstance(raw_auth, str):
+ try:
+ auth_config: dict[str, Any] = json.loads(raw_auth)
+ except json.JSONDecodeError as e:
+ raise ValueError(f"Failed to parse auth configuration as JSON: {raw_auth!r}") from e
+ else:
+ auth_config = raw_auth
auth_type = auth_config.get("type")
Review Comment:
This is now handled! After decoding, the code checks that the result is a dictionary and raises a ValueError if not. Thanks for the suggestion!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "GayathriSrividya (via GitHub)" <gi...@apache.org>.
GayathriSrividya commented on code in PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#discussion_r3324290470
##########
tests/catalog/test_rest.py:
##########
@@ -3167,3 +3167,135 @@ def test_load_table_without_storage_credentials(
)
assert actual.metadata.model_dump() == expected.metadata.model_dump()
assert actual == expected
+
+
+# Tests for issue #3422: REST catalog auth cannot be configured via environment
+# variables unless auth JSON strings are decoded.
+
+
+def test_rest_catalog_with_basic_auth_as_json_string(rest_mock: Mocker) -> None:
Review Comment:
This is now addressed! The tests for REST catalog auth via JSON string and flat properties are parameterized and cover:
- Basic, OAuth2 (including int fields and list scopes), Google, and Entra auth types
- Edge cases like list[str] scopes and integer fields
This makes it easy to extend and ensures all parsing scenarios are tested, as you suggested. Thank you!
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] fix: support env-var auth config for REST catalog (JSON string decode + flat properties) [iceberg-python]
Posted by "rambleraptor (via GitHub)" <gi...@apache.org>.
rambleraptor commented on code in PR #3423:
URL: https://github.com/apache/iceberg-python/pull/3423#discussion_r3351588627
##########
pyiceberg/catalog/rest/__init__.py:
##########
@@ -435,7 +436,32 @@ def _create_session(self) -> Session:
elif ssl_client_cert := ssl_client.get(CERT):
session.cert = ssl_client_cert
- if auth_config := self.properties.get(AUTH):
+ raw_auth = self.properties.get(AUTH)
+ if isinstance(raw_auth, str):
+ try:
+ auth_config: dict[str, Any] | None = json.loads(raw_auth)
+ except json.JSONDecodeError as e:
+ raise ValueError("Failed to parse auth configuration as JSON") from e
+ elif raw_auth is not None:
+ auth_config = raw_auth
+ elif auth_type := self.properties.get(f"{AUTH}.type"):
+ type_prefix = f"{AUTH}.{auth_type}."
+ auth_config = {
Review Comment:
Thought: Right now, properties is a dict[str, Any], so auth_config is a `dict[str, Any]`. We should have a type signature for the different parts. I'd love to have type checking on these auth properties.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org