Skip to content
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

Schemaregistrytimeoutoption #906

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions confluent_kafka/schema_registry/schema_registry_client.py
Expand Up @@ -95,7 +95,7 @@ def __init__(self, conf):
raise ValueError("ssl.certificate.location required when"
" configuring ssl.key.location")

self.timeout = conf_copy.pop('timeout', 30.00)
self.timeout = conf_copy.pop('request.timeout', 30.00)

userinfo = utils.get_auth_from_url(base_url)
if 'basic.auth.user.info' in conf_copy:
Expand Down Expand Up @@ -282,7 +282,12 @@ class SchemaRegistryClient(object):
| | | By default userinfo is extracted from |
| | | the URL if present. |
+------------------------------+------+-------------------------------------------------+

| | | Timeout for HTTP/S calls to Schema registry |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Timeout in seconds for ..

| | | |
| ``request.timeout`` |float | |
| | | By default timeout is 30.00 seconds |
| | | the URL if present. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the URL if present." should probably be removed.

+------------------------------+------+-------------------------------------------------+
Args:
conf (dict): Schema Registry client configuration.

Expand Down
11 changes: 11 additions & 0 deletions tests/schema_registry/test_api_client.py
Expand Up @@ -21,6 +21,7 @@

from confluent_kafka.schema_registry.error import SchemaRegistryError
from confluent_kafka.schema_registry.schema_registry_client import Schema
from urllib3.exceptions import ConnectTimeoutError

"""
Basic SchemaRegistryClient API functionality tests.
Expand Down Expand Up @@ -376,3 +377,13 @@ def test_schema_equivilence(load_avsc):
assert schema == schema2
assert schema_str1.__eq__(schema_str2)
assert schema_str1 == schema_str2

def test_timeout_exception_thrown_when_unreasonably_low_timeout_set(mock_schema_registry):
conf = {'url': TEST_URL,
'request.timeout' : 0.01
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if TEST_URL is a valid URL I don't think a request timeout of 10ms is sufficienctly reliable to be exceeded.
Instead suggest creating a TCP listener that you connect to.

}
sr = mock_schema_registry(conf)
with pytest.raises(ConnectTimeoutError) as e :
sr.get_schema()
assert isinstance(e, ConnectTimeoutError)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this assert needed? I believe pytest.raises already check this