ZMQ example using python3 and asyncio #9485
Conversation
|
What about "rewriting" the current python2 zmq example instead of creating a new one for python3? I don't think we need a python2 example. |
|
For nearly a decade people have been "avoiding" upgrading to python 3. I don't know what's wrong with these people. They should probably see a psychologist and get a friendly lobotomy. Obviously I think python3 and asyncio is the "right way" to do anything. It has all the advantages of javascript's asynchronicity and event loop while not being a totally brain-dead language. TL;DR I'd be very happy to replace the current example with this python3 one...but people who need friendly lobotomies will complain. |
|
Maybe mention that the old version can still be retrieved from git history? |
|
Agree with re-writing to use Python3, I don't think we need to keep examples for both in the repo. |
|
Agreed that a Python3 version is preferable. Python2 support for ZMQ libraries > 3 is uncertain and attempts to script for both Py2 and Py3 will run into incompatibilities of hex encoding, as well as pickle protocol limitations in Py2's pyzmq module. |
|
Since everyone here likes python3, I replaced the python2.7 file with the python3 example, and included a link to the python2.7 example in the git history. I also removed the Polling wrapper class from the example, in favor of more direct use of asyncio primitives. Again it's functionally identical to the original example. |
| self.zmqSubSocket.setsockopt_string(zmq.SUBSCRIBE, "rawtx") | ||
| self.zmqSubSocket.connect("tcp://127.0.0.1:%i" % port) | ||
|
|
||
| async def handle(self) : |
There was a problem hiding this comment.
This function definition syntax is specific to Python 3.5 and throws an exception in Python 3.4. Perhaps a from __future__ import option exists that will allow compatibility with all Python 3 minor versions?
There was a problem hiding this comment.
Asyncio was only introduced in 3.5 including these syntax enhancements and was "provisional". It's now final that 3.6 was released a week ago, and backported (as final) to 3.5 point releases. 3.4 doesn't have it and never will. But, it doesn't have anywhere near the user base of 2.7 and I don't think is worth supporting long term.
I tried to install 3.4 while testing this, it's not even provided anymore by Ubuntu.
There was a problem hiding this comment.
FYI the "yield from" syntax of 3.4 has been entirely deprecated.
There was a problem hiding this comment.
Useful to know and thanks for the info & link @mcelrath
Python 3.5 will undoubtedly become the de facto standard, as you imply. In providing example scripts it seems reasonable that Bitcoin cater to the common denominator. It's probably safe to assume that most nodes out there are not running on the latest OS versions and that Ubuntu 14.04 and even 12.04 (in some cases) are the majority. From this perspective catering for Python 3.4 is useful since it is both repository standard and supported.
Your 3.5 approach using asyncio is great. My comments are really just thinking about ease of use and compatibility, hence the suggestion to cater for 3.4 and the inevitable 3.5 future
There was a problem hiding this comment.
Ubuntu 14.04 LTS is supported until April 2019, and currently ships Python 3.4.3, so I also think it makes sense to provide examples compatible with Python 3.4.
|
Python 3.4 seems to be widely used while Python 3.5 is still pushing out the boat. Deprecating Python 2 in favor of version 3 has many benefits, yet, thinking about general usability, it would perhaps make sense to keep this script's functionality dependent on the zmq module and avoid adding asyncio dependency? Don't get me wrong - the use of asyncio in this script is clean and efficient - but it adds a dependency; an extra layer; and the need for the user (seeking a ZMQ example) to be familiar with asyncio (and its usage in Python 3.5). I suggest having two example scripts: a vanilla ZMQ SUBscriber compatible with Python 3.4 (and based on the previous 2.7 script) as well as the current (3.5) script using asyncio and compatible with Python 3.4, if possible. |
|
Bah, I think we should support 3.4 for the forseeable future. I don't care about 2.x compatibility. Can't we make asyncio usage dependent on Python version or something? Is it hard to make a fallback, to support both, use the fancy new i/o system on 3.5+? |
|
See I told ya this would result in a python version fight...I just didn't anticipate anyone wanting 3.4! :-P Python 3.4 was last released in Ubuntu in version 14.04. It has been excluded in 14.10, 15.04, 15.10, 16.04 LTS, and 16.10. It's ancient history. The python project itself has designated 3.4 as "security fixes only" and is no longer producing binaries for it. The syntax change is, unfortunately, not backwards compatible, and results in a syntax error on 3.4. Consequently there is no I literally don't have any machines at my disposal with python3.4 on them. I added a second example which should work on both python 3.4 and 3.5. Could one of you 3.4 aficionados test it please? |
|
Concept ACK, cannot test this right now, may do so later (I do have an Ubuntu Trusty/14.04 instance to test, note that we also use this version of Ubuntu on Travis and to build gitian releases). |
|
Needs rebase. Maybe also squash the first two commits? |
|
Rebased. I usually squash when merging but I can squash ahead of time if desired. |
There was a problem hiding this comment.
Looks great. I've tested this with Python3.5 and it works really nicely.
I've added 3 μ-nits.
Personally I don't agree that we should have two almost identical versions of this in the repository, especially since the /contrib code is not run as part of the build process. If someone really wants to run this on Python3.4, they should make the syntax changes themselves.
| @@ -1,43 +1,70 @@ | |||
| #!/usr/bin/env python2 | |||
| #!/usr/bin/env python3 | |||
| # Copyright (c) 2014-2016 The Bitcoin Core developers | |||
| # Distributed under the MIT software license, see the accompanying | |||
| # file COPYING or http://www.opensource.org/licenses/mit-license.php. | |||
|
|
|||
There was a problem hiding this comment.
nit: I'd like to see a module-level docstring describing what this does (which can include the comment block below about the pre-2.7 version)
contrib/zmq/zmq_sub.py
Outdated
| # A blocking example using python 2.7 can be obtained from the git history: | ||
| # https://github.com/bitcoin/bitcoin/blob/37a7fe9e440b83e2364d5498931253937abe9294/contrib/zmq/zmq_sub.py | ||
|
|
||
| import array |
There was a problem hiding this comment.
nit: I don't think this import array is actually used?
contrib/zmq/zmq_sub.py
Outdated
| import binascii | ||
| import zmq | ||
| import asyncio, zmq, zmq.asyncio |
There was a problem hiding this comment.
nit: prefer imports to be on individual line (PEP8 E401), and listed individually.
|
@jnewbery Well the point of |
|
@mcelrath Sure. I'm not massively concerned about what goes on in /contrib. Having two versions of zmq_sub.py just makes this marginally less maintainable, since any changes to the code would need to be repeated and tested in two places against two different versions of python. Perhaps adding a comment to the top of the module with a link to the python docs about asyncio syntax differences is a better way to go. Anyway, like I said, I'm not hugely concerned either way. Tested ACK. |
|
Looks great. Tested ACK b471daf |
|
ACK |
b471daf Adddress nits, use asyncio signal handling, create_task (Bob McElrath) 4bb7d1b Add python version checks and 3.4 example (Bob McElrath) 5406d51 Rewrite to not use Polling wrapper for asyncio, link to python2.7 example (Bob McElrath) 5ea5368 ZMQ example using python3 and asyncio (Bob McElrath)
b471daf Adddress nits, use asyncio signal handling, create_task (Bob McElrath) 4bb7d1b Add python version checks and 3.4 example (Bob McElrath) 5406d51 Rewrite to not use Polling wrapper for asyncio, link to python2.7 example (Bob McElrath) 5ea5368 ZMQ example using python3 and asyncio (Bob McElrath)
* Merge bitcoin#9744: Remove unused module from rpc-tests a432aa0 Remove unused module from rpc-tests (Takashi Mitsuta) * Merge bitcoin#9696: [trivial] Fix recently introduced typos in comments 0c9b9b7 [trivial] Fix recently introduced typos in comments (practicalswift) * Merge bitcoin#9657: Improve rpc-tests.py a6a3e58 Various review markups for rpc-tests.py improvements (John Newbery) 3de3ccd Refactor rpc-tests.py (John Newbery) afd38e7 Improve rpc-tests.py arguments (John Newbery) 91bffff Use argparse in rpc_tests.py (John Newbery) 1581ecb Use configparser in rpc-tests.py (John Newbery) * Merge bitcoin#9724: Qt/Intro: Add explanation of IBD process f6d18f5 Qt/Intro: Explain a bit more what will happen first time (Luke Dashjr) 50c5657 Qt/Intro: Storage shouldn't grow significantly with pruning enabled (Luke Dashjr) 9adb694 Qt/Intro: Move sizeWarningLabel text into C++ code (Luke Dashjr) * Merge bitcoin#9794: Minor update to qrencode package builder 1bfe6b4 Use package name variable inside $(package)_file_name variable (Mitchell Cash) * Merge bitcoin#9726: netbase: Do not print an error on connection timeouts through proxy 3ddfe29 netbase: Do not print an error on connection timeouts through proxy (Wladimir J. van der Laan) 13f6085 netbase: Make InterruptibleRecv return an error code instead of bool (Wladimir J. van der Laan) * Merge bitcoin#9727: Remove fallbacks for boost_filesystem < v3 056aba2 Remove fallbacks for boost_filesystem < v3 (Wladimir J. van der Laan) * Merge bitcoin#9485: ZMQ example using python3 and asyncio b471daf Adddress nits, use asyncio signal handling, create_task (Bob McElrath) 4bb7d1b Add python version checks and 3.4 example (Bob McElrath) 5406d51 Rewrite to not use Polling wrapper for asyncio, link to python2.7 example (Bob McElrath) 5ea5368 ZMQ example using python3 and asyncio (Bob McElrath) * Merge bitcoin#9807: RPC doc fix-ups. 851f6a3 [qa][doc] Correct rpc test options in readme (fanquake) 41e7219 [trivial] Add tests_config.ini to .gitignore (fanquake) * Dashify Co-Authored-By: PastaPastaPasta <pasta@dashboost.org> * Change file permissions * update travis.yml -parallel -> --jobs
b471daf Adddress nits, use asyncio signal handling, create_task (Bob McElrath) 4bb7d1b Add python version checks and 3.4 example (Bob McElrath) 5406d51 Rewrite to not use Polling wrapper for asyncio, link to python2.7 example (Bob McElrath) 5ea5368 ZMQ example using python3 and asyncio (Bob McElrath)
b471daf Adddress nits, use asyncio signal handling, create_task (Bob McElrath) 4bb7d1b Add python version checks and 3.4 example (Bob McElrath) 5406d51 Rewrite to not use Polling wrapper for asyncio, link to python2.7 example (Bob McElrath) 5ea5368 ZMQ example using python3 and asyncio (Bob McElrath)
b471daf Adddress nits, use asyncio signal handling, create_task (Bob McElrath) 4bb7d1b Add python version checks and 3.4 example (Bob McElrath) 5406d51 Rewrite to not use Polling wrapper for asyncio, link to python2.7 example (Bob McElrath) 5ea5368 ZMQ example using python3 and asyncio (Bob McElrath)
* Merge bitcoin#9744: Remove unused module from rpc-tests a432aa0 Remove unused module from rpc-tests (Takashi Mitsuta) * Merge bitcoin#9696: [trivial] Fix recently introduced typos in comments 0c9b9b7 [trivial] Fix recently introduced typos in comments (practicalswift) * Merge bitcoin#9657: Improve rpc-tests.py a6a3e58 Various review markups for rpc-tests.py improvements (John Newbery) 3de3ccd Refactor rpc-tests.py (John Newbery) afd38e7 Improve rpc-tests.py arguments (John Newbery) 91bffff Use argparse in rpc_tests.py (John Newbery) 1581ecb Use configparser in rpc-tests.py (John Newbery) * Merge bitcoin#9724: Qt/Intro: Add explanation of IBD process f6d18f5 Qt/Intro: Explain a bit more what will happen first time (Luke Dashjr) 50c5657 Qt/Intro: Storage shouldn't grow significantly with pruning enabled (Luke Dashjr) 9adb694 Qt/Intro: Move sizeWarningLabel text into C++ code (Luke Dashjr) * Merge bitcoin#9794: Minor update to qrencode package builder 1bfe6b4 Use package name variable inside $(package)_file_name variable (Mitchell Cash) * Merge bitcoin#9726: netbase: Do not print an error on connection timeouts through proxy 3ddfe29 netbase: Do not print an error on connection timeouts through proxy (Wladimir J. van der Laan) 13f6085 netbase: Make InterruptibleRecv return an error code instead of bool (Wladimir J. van der Laan) * Merge bitcoin#9727: Remove fallbacks for boost_filesystem < v3 056aba2 Remove fallbacks for boost_filesystem < v3 (Wladimir J. van der Laan) * Merge bitcoin#9485: ZMQ example using python3 and asyncio b471daf Adddress nits, use asyncio signal handling, create_task (Bob McElrath) 4bb7d1b Add python version checks and 3.4 example (Bob McElrath) 5406d51 Rewrite to not use Polling wrapper for asyncio, link to python2.7 example (Bob McElrath) 5ea5368 ZMQ example using python3 and asyncio (Bob McElrath) * Merge bitcoin#9807: RPC doc fix-ups. 851f6a3 [qa][doc] Correct rpc test options in readme (fanquake) 41e7219 [trivial] Add tests_config.ini to .gitignore (fanquake) * Dashify Co-Authored-By: PastaPastaPasta <pasta@dashboost.org> * Change file permissions * update travis.yml -parallel -> --jobs
As is says on the tin. This example is functionally identical to the polling 2.7 zmq example, but uses the fancy new non-blocking asyncio of python 3.5.