Skip to content

Catch exception if conneciton is None#857

Closed
beltran wants to merge 2 commits into
masterfrom
python-check-connection
Closed

Catch exception if conneciton is None#857
beltran wants to merge 2 commits into
masterfrom
python-check-connection

Conversation

@beltran

@beltran beltran commented Nov 13, 2017

Copy link
Copy Markdown
Contributor

No description provided.

@beltran beltran force-pushed the python-check-connection branch from 049a1c0 to ae64d1d Compare November 14, 2017 09:24
@mambocab

Copy link
Copy Markdown
Contributor

Is a request ID issued to the request by the time this happens? I don't see the exact sequence of events, but I wonder if this leads to races where a request ID never gets returned to the pool of available ones.

@beltran

beltran commented Nov 28, 2017

Copy link
Copy Markdown
Contributor Author

That's a good point, we cannot return the connection because it hasn't been borrowed yet. It won't necessarily be lost since it will be returned when the response comes back. Ideally we would cancel sending it at this point but that doesn't seem easy.

@mambocab mambocab force-pushed the python-check-connection branch from 0868bf7 to ae64d1d Compare January 3, 2018 19:45
@mambocab

mambocab commented Jan 3, 2018

Copy link
Copy Markdown
Contributor

Pushed up a changelog update. We'll need to rebase onto master before merge, but I just want to see what the unit tests look like.

But I think I agree, this doesn't solve the issue -- if self._connection can be None at self._connection._requests.pop(self._req_id), it can be None at with self._connection.lock:, right?

Maybe what we want is a rescheduling solution -- I'll try something like

if self._connection is None:
    self.session.cluster.connection_class.create_timer(0.01, self._on_timeout)
    return

@beltran

beltran commented May 8, 2018

Copy link
Copy Markdown
Contributor Author

Seems like this was solved with #899 so closing this.

@beltran beltran closed this May 8, 2018
@mambocab mambocab deleted the python-check-connection branch May 9, 2018 19:06
@mambocab

mambocab commented May 9, 2018

Copy link
Copy Markdown
Contributor

Awesome, thanks for the cleanup.

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.

2 participants