You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "MenelikV (via GitHub)" <gi...@apache.org> on 2026/02/09 15:32:52 UTC
[PR] fix ssl deactivation from catalog properties #2985 [iceberg-python]
MenelikV opened a new pull request, #3012:
URL: https://github.com/apache/iceberg-python/pull/3012
<!--
Thanks for opening a pull request!
-->
<!-- In the case this PR will resolve an issue, please replace ${GITHUB_ISSUE_ID} below with the actual Github issue id. -->
Closes #2985
# Rationale for this change
## Are these changes tested?
Yes
## Are there any user-facing changes?
<!-- In the case of user-facing changes, please add the changelog label. -->
--
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 ssl deactivation from catalog properties #2985 [iceberg-python]
Posted by "MenelikV (via GitHub)" <gi...@apache.org>.
MenelikV commented on code in PR #3012:
URL: https://github.com/apache/iceberg-python/pull/3012#discussion_r2786614487
##########
tests/catalog/test_rest.py:
##########
@@ -1641,6 +1641,29 @@ def test_update_namespace_properties_invalid_namespace(rest_mock: Mocker) -> Non
assert "Empty namespace identifier" in str(e.value)
+def test_with_disabled_ssl_ca_bundle(rest_mock: Mocker) -> None:
+ from pydantic import ValidationError
Review Comment:
With the simpler test, I don't need this import anymore, I will mark this as resolved.
--
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 ssl deactivation from catalog properties #2985 [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on code in PR #3012:
URL: https://github.com/apache/iceberg-python/pull/3012#discussion_r2809966815
##########
pyiceberg/catalog/rest/__init__.py:
##########
@@ -358,7 +358,7 @@ def _create_session(self) -> Session:
# Sets the client side and server side SSL cert verification, if provided as properties.
if ssl_config := self.properties.get(SSL):
- if ssl_ca_bundle := ssl_config.get(CA_BUNDLE):
+ if (ssl_ca_bundle := ssl_config.get(CA_BUNDLE)) is not None:
session.verify = ssl_ca_bundle
Review Comment:
Nit:
```suggestion
if ssl_config.get(CA_BUNDLE) is not None:
session.verify = ssl_config.get[CA_BUNDLE]
```
--
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 ssl deactivation from catalog properties #2985 [iceberg-python]
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #3012:
URL: https://github.com/apache/iceberg-python/pull/3012
--
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 ssl deactivation from catalog properties #2985 [iceberg-python]
Posted by "MenelikV (via GitHub)" <gi...@apache.org>.
MenelikV commented on code in PR #3012:
URL: https://github.com/apache/iceberg-python/pull/3012#discussion_r2786577185
##########
tests/catalog/test_rest.py:
##########
@@ -1641,6 +1641,29 @@ def test_update_namespace_properties_invalid_namespace(rest_mock: Mocker) -> Non
assert "Empty namespace identifier" in str(e.value)
+def test_with_disabled_ssl_ca_bundle(rest_mock: Mocker) -> None:
+ from pydantic import ValidationError
Review Comment:
Do you want me to move up the import ? I thought I could be confusing with pyiceberg own ValidationError :/
--
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 ssl deactivation from catalog properties #2985 [iceberg-python]
Posted by "MenelikV (via GitHub)" <gi...@apache.org>.
MenelikV commented on code in PR #3012:
URL: https://github.com/apache/iceberg-python/pull/3012#discussion_r2786586561
##########
tests/catalog/test_rest.py:
##########
@@ -1641,6 +1641,29 @@ def test_update_namespace_properties_invalid_namespace(rest_mock: Mocker) -> Non
assert "Empty namespace identifier" in str(e.value)
+def test_with_disabled_ssl_ca_bundle(rest_mock: Mocker) -> None:
+ from pydantic import ValidationError
+ def ssl_check_callback(req, _):
+ if req.verify:
+ raise AssertionError("SSL verification is still enabled")
+ # Given
+ rest_mock.get(
+ f"{TEST_URI}v1/config",
+ json=ssl_check_callback,
+ status_code=200,
+ )
+ # Given
+ catalog_properties = {
+ "uri": TEST_URI,
+ "token": TEST_TOKEN,
+ "ssl": {
+ "cabundle": False,
+ }
+ }
+ with pytest.raises(ValidationError) as _:
Review Comment:
Okay, works for me.
--
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