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

Fix lack of timeout support in KafkaClient and KafkaConnection #161

Merged
merged 1 commit into from
May 6, 2014

Conversation

maciejkula
Copy link
Contributor

Timeout was not passed correctly from KafkaClient to KafkaConnection --- and then was not actually applied to the socket in KafkaConnection itself.

To test this, try KafkaClient('1.2.3.4:1234', timeout=0.1) --- this hangs for a long time in master (as does KafkaConnection('1.2.3.4', 1234, 0.1).

This is a critical issue in production systems.

@wizzat
Copy link
Collaborator

wizzat commented Apr 19, 2014

FWIW, I spotted this in #158 and fixed it there as well. I didn't want to write a special test for the connect method so I just refactored it away in favor of the correct one.

@maciejkula
Copy link
Contributor Author

Sure, I saw that, looks good --- but I don't think you fixed the fact that settimeout is called after the connection is made? In conn.py:

    self._sock.connect((self.host, self.port))
    self._sock.settimeout(self.timeout)

wizzat added a commit to wizzat/kafka-python that referenced this pull request Apr 19, 2014
@wizzat
Copy link
Collaborator

wizzat commented Apr 19, 2014

Oh, you are totally correct. I pulled that in and wrote a test for it so that it doesn't get broken in the future.

@maciejkula
Copy link
Contributor Author

Sounds good. When do you think you will merge into master?

dpkp added a commit that referenced this pull request May 6, 2014
Fix connection timeout in KafkaClient and KafkaConnection
@dpkp dpkp merged commit 914c2e6 into dpkp:master May 6, 2014
wbarnha added a commit to orange-kao/kafka-python that referenced this pull request Mar 9, 2024
* Test Kafka 0.8.2.2 using Python 3.11 in the meantime

* Override PYTHON_LATEST conditionally in python-package.yml

* Update python-package.yml

* add python annotation to kafka version test matrix

* Update python-package.yml

* try python 3.10
bradenneal1 pushed a commit to bradenneal1/kafka-python that referenced this pull request May 16, 2024
* Test Kafka 0.8.2.2 using Python 3.11 in the meantime

* Override PYTHON_LATEST conditionally in python-package.yml

* Update python-package.yml

* add python annotation to kafka version test matrix

* Update python-package.yml

* try python 3.10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants