- Notifications
You must be signed in to change notification settings - Fork 30
release: v0.9.6 #223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
release: v0.9.6 #223
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
0.9.5 | ||
0.9.6 |
Original file line number | Diff line number | Diff line change |
---|---|---|
| @@ -535,8 +535,8 @@ def release_conn(self): | |
| ||
| ||
# Tests for SSL Context Reuse (fix for OpenSSL 3.0+ performance issues) | ||
@patch('ssl.create_default_context') | ||
@patch('urllib3.PoolManager') | ||
@patch("ssl.create_default_context") | ||
@patch("urllib3.PoolManager") | ||
def test_ssl_context_created_with_ca_cert(mock_pool_manager, mock_create_context): | ||
"""Test that SSL context is created with CA certificate file.""" | ||
mock_ssl_context = MagicMock() | ||
| @@ -559,11 +559,11 @@ def test_ssl_context_created_with_ca_cert(mock_pool_manager, mock_create_context | |
# Verify SSL context was passed to PoolManager | ||
mock_pool_manager.assert_called_once() | ||
call_kwargs = mock_pool_manager.call_args[1] | ||
assert call_kwargs['ssl_context'] == mock_ssl_context | ||
assert call_kwargs["ssl_context"] == mock_ssl_context | ||
| ||
| ||
@patch('ssl.create_default_context') | ||
@patch('urllib3.PoolManager') | ||
@patch("ssl.create_default_context") | ||
@patch("urllib3.PoolManager") | ||
def test_ssl_context_loads_client_certificate(mock_pool_manager, mock_create_context): | ||
"""Test that SSL context loads client certificate and key when provided.""" | ||
mock_ssl_context = MagicMock() | ||
| @@ -591,12 +591,14 @@ def test_ssl_context_loads_client_certificate(mock_pool_manager, mock_create_con | |
# Verify SSL context was passed to PoolManager | ||
mock_pool_manager.assert_called_once() | ||
call_kwargs = mock_pool_manager.call_args[1] | ||
assert call_kwargs['ssl_context'] == mock_ssl_context | ||
assert call_kwargs["ssl_context"] == mock_ssl_context | ||
| ||
| ||
@patch('ssl.create_default_context') | ||
@patch('urllib3.PoolManager') | ||
def test_ssl_context_disables_verification_when_verify_ssl_false(mock_pool_manager, mock_create_context): | ||
@patch("ssl.create_default_context") | ||
@patch("urllib3.PoolManager") | ||
def test_ssl_context_disables_verification_when_verify_ssl_false( | ||
mock_pool_manager, mock_create_context | ||
): | ||
"""Test that SSL context disables verification when verify_ssl=False.""" | ||
mock_ssl_context = MagicMock() | ||
mock_create_context.return_value = mock_ssl_context | ||
| @@ -622,11 +624,11 @@ def test_ssl_context_disables_verification_when_verify_ssl_false(mock_pool_manag | |
# Verify SSL context was passed to PoolManager | ||
mock_pool_manager.assert_called_once() | ||
call_kwargs = mock_pool_manager.call_args[1] | ||
assert call_kwargs['ssl_context'] == mock_ssl_context | ||
assert call_kwargs["ssl_context"] == mock_ssl_context | ||
| ||
| ||
@patch('ssl.create_default_context') | ||
@patch('urllib3.ProxyManager') | ||
@patch("ssl.create_default_context") | ||
@patch("urllib3.ProxyManager") | ||
def test_ssl_context_used_with_proxy_manager(mock_proxy_manager, mock_create_context): | ||
"""Test that SSL context is passed to ProxyManager when proxy is configured.""" | ||
mock_ssl_context = MagicMock() | ||
| @@ -655,14 +657,16 @@ def test_ssl_context_used_with_proxy_manager(mock_proxy_manager, mock_create_con | |
# Verify SSL context was passed to ProxyManager | ||
mock_proxy_manager.assert_called_once() | ||
call_kwargs = mock_proxy_manager.call_args[1] | ||
assert call_kwargs['ssl_context'] == mock_ssl_context | ||
assert call_kwargs['proxy_url'] == "http://proxy:8080" | ||
assert call_kwargs['proxy_headers'] == {"Proxy-Auth": "token"} | ||
assert call_kwargs["ssl_context"] == mock_ssl_context | ||
assert call_kwargs["proxy_url"] == "http://proxy:8080" | ||
assert call_kwargs["proxy_headers"] == {"Proxy-Auth": "token"} | ||
| ||
| ||
@patch('ssl.create_default_context') | ||
@patch('urllib3.PoolManager') | ||
def test_ssl_context_reuse_performance_optimization(mock_pool_manager, mock_create_context): | ||
@patch("ssl.create_default_context") | ||
@patch("urllib3.PoolManager") | ||
def test_ssl_context_reuse_performance_optimization( | ||
mock_pool_manager, mock_create_context | ||
): | ||
"""Test that SSL context creation is called only once per client instance.""" | ||
mock_ssl_context = MagicMock() | ||
mock_create_context.return_value = mock_ssl_context | ||
| @@ -685,7 +689,7 @@ def test_ssl_context_reuse_performance_optimization(mock_pool_manager, mock_crea | |
# Verify the same SSL context instance is reused | ||
mock_pool_manager.assert_called_once() | ||
call_kwargs = mock_pool_manager.call_args[1] | ||
assert call_kwargs['ssl_context'] is mock_ssl_context | ||
assert call_kwargs["ssl_context"] is mock_ssl_context | ||
| ||
# Verify context was not created again during subsequent operations | ||
mock_create_context.reset_mock() | ||
| @@ -697,8 +701,8 @@ def test_ssl_context_reuse_performance_optimization(mock_pool_manager, mock_crea | |
mock_create_context.assert_not_called() | ||
| ||
| ||
@patch('ssl.create_default_context') | ||
@patch('urllib3.PoolManager') | ||
@patch("ssl.create_default_context") | ||
@patch("urllib3.PoolManager") | ||
Comment on lines +704 to +705 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fragile assertion: MagicMock makes the "secure defaults not modified" check vacuous. hasattr() on MagicMock returns True and attributes are truthy Mocks, so the assertions would pass even if code regressed. Assert concrete values instead and pre-set expected defaults. Apply this diff to make the assertions meaningful: - # Verify SSL verification settings were NOT modified (verify_ssl=True) - # check_hostname and verify_mode should remain at their default secure values - assert ( - not hasattr(mock_ssl_context, "check_hostname") - or mock_ssl_context.check_hostname - ) - assert ( - not hasattr(mock_ssl_context, "verify_mode") - or mock_ssl_context.verify_mode != ssl.CERT_NONE - ) + # Verify SSL verification settings were NOT modified (verify_ssl=True) + assert mock_ssl_context.check_hostname is True + assert mock_ssl_context.verify_mode == ssl.CERT_REQUIRED And initialize the defaults on the mocked context (outside the changed lines, right after creating mock_ssl_context): # after: mock_ssl_context = MagicMock() mock_ssl_context.check_hostname = True mock_ssl_context.verify_mode = ssl.CERT_REQUIRED Also applies to: 732-739, 744-745 🤖 Prompt for AI Agents
| ||
def test_ssl_context_with_all_ssl_options(mock_pool_manager, mock_create_context): | ||
"""Test SSL context creation with all SSL configuration options set.""" | ||
mock_ssl_context = MagicMock() | ||
| @@ -725,11 +729,17 @@ def test_ssl_context_with_all_ssl_options(mock_pool_manager, mock_create_context | |
| ||
# Verify SSL verification settings were NOT modified (verify_ssl=True) | ||
# check_hostname and verify_mode should remain at their default secure values | ||
assert not hasattr(mock_ssl_context, 'check_hostname') or mock_ssl_context.check_hostname | ||
assert not hasattr(mock_ssl_context, 'verify_mode') or mock_ssl_context.verify_mode != ssl.CERT_NONE | ||
assert ( | ||
not hasattr(mock_ssl_context, "check_hostname") | ||
or mock_ssl_context.check_hostname | ||
) | ||
assert ( | ||
not hasattr(mock_ssl_context, "verify_mode") | ||
or mock_ssl_context.verify_mode != ssl.CERT_NONE | ||
) | ||
| ||
# Verify SSL context was passed to PoolManager | ||
mock_pool_manager.assert_called_once() | ||
call_kwargs = mock_pool_manager.call_args[1] | ||
assert call_kwargs['ssl_context'] == mock_ssl_context | ||
assert call_kwargs['maxsize'] == 8 | ||
assert call_kwargs["ssl_context"] == mock_ssl_context | ||
assert call_kwargs["maxsize"] == 8 |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
🧩 Analysis chain
DEFAULT_USER_AGENT updated — good.
To prevent future PR churn, derive from version.
Apply:
Add (outside this hunk) near imports:
Check all UA strings reference the current version:
🏁 Script executed:
Length of output: 174
DEFAULT_USER_AGENT updated — good. Remaining hardcoded User-Agent strings found; update them to derive from version.
Apply (api_client.py):
Add near imports:
Files to fix (from verification):
Replace:
with:
and add
from openfga_sdk import __version__
to those modules' imports.🤖 Prompt for AI Agents