tornado 5.x support #232
tornado 5.x support #232
Conversation
|
note that I removed nsq-0.3.8 |
|
half of |
For what it's worth, my last tornado upgrade effort only got me as far as |
|
hah, so does that mean we should try for compatibility down to tornado-4.0.z ... I just found |
|
I mean if 4.1+ doesn't cause any specific complications i'd appreciate it. |
|
I think this is feature-complete. Please take a look. If the direction looks good, I'll squash down to about 4 commits, and try to actually use it ... |
|
|
||
| self.stream = tornado.iostream.SSLIOStream(self.socket, io_loop=self.io_loop) | ||
| self.stream.set_close_callback(self._socket_close) | ||
| fut = self.stream.start_tls(False, ssl_options=opts, server_hostname=self.host) |
There was a problem hiding this comment.
I don't think we were checking if the TLS server certificate was valid for the hostname connected to. Now, if not ssl.CERT_NONE, this will (I think) check hostname ...
|
mostly squashed |
|
I threw up a version bump to 0.8.4b1 because it makes testing with my systems a bit easier. |
|
I'm finally running this for my staging services ... it seems to be working just fine. But note that I don't really use compression or tls features. |
| @@ -24,6 +24,17 @@ def recv(self, size): | |||
| def read(self, size): | |||
| return self._recv(size, self._socket.read) | |||
|
|
|||
| def recv_into(self, buf, nbytes=0): | |||
There was a problem hiding this comment.
why is this method necessary?
There was a problem hiding this comment.
it's the socket method which tornado-5.x uses instead of recv(), for potential efficiency improvement: tornadoweb/tornado@1215cd2
|
(I'll re-squash down to 3 or 4 commits, when ready) |
|
cool, error handling improvements look good 👍🏻 🔨 |
tornado 5.x compatibility to be corrected in the following commits also test with tornado 4.1 for bitly (it's not yet a real burden)
no longer pass io_loop to tornado object contructors no longer accept io_loop in nsq.Client subclasses adapt tests for removed io_loop kwarg no longer get "default certs" from tornado, no longer need certifi, instead use IOStream.start_tls() (which uses python SSLContext stuff to get default CA root bundle) start passing hostname for checking in tls certificate
also test Deflate without SSL so we have some reader tests without SSL the bootstrap thing has a minor possible issue with fd-readyness but let's not fix that now (seems to have worked for a while anyway)
|
mostly squashed |
addresses #185 and #203
still adapting tests ... they passed in a mocked io_loop so it's a pretty big change if they can't do thatadapted