Use ZeroMQ message bus in lieu of build queue using new BuildManager #29
Conversation
mpacer
force-pushed
the
zmq_bus_build_manager
branch
9 times, most recently
from
a56ba17
to
2034f0f
Compare
mpacer
force-pushed
the
zmq_bus_build_manager
branch
from
92e15f1
to
7930574
Compare
- rename package_dir --> package_path - remove unused imports from builder, utils, pr_list.__init__, - move cache from builder to pr_list - move status_file from utils to pr_list - move status_from_cache from server to pr_list - change outdated_pr_list --> update_pr_list
mpacer
force-pushed
the
zmq_bus_build_manager
branch
from
7930574
to
9c9811b
Compare
mpacer
force-pushed
the
zmq_bus_build_manager
branch
from
638d04e
to
a955b28
Compare
Use nr not pr to refer to numbers
|
@stefanv is there anything else that we should accomplish in this PR? |
- describe how to get the latest version of the scipy_proc - describe where to go to install LaTeX - remove sudo from apt-get - explicitly point out that that will only work if you have access to `apt-get`
It is nice to keep the message printing around for debugging when more than one kind of message can be sent through.
mpacer
changed the title
|
@stefanv I think that this is good to go should I merge it? |
procbuild/listener.py
Outdated
| class Listener: | ||
| """ Listener class for defining zmq sockets and maintaining a build queue. | ||
|
|
||
| Attributes: |
There was a problem hiding this comment.
Let's use the NumPy documentation format, also below.
procbuild/listener.py
Outdated
| """ | ||
| while True: | ||
| msg = await self.socket.recv_multipart() | ||
| target, raw_payload = msg |
There was a problem hiding this comment.
Use consistent naming for prefix or target or channel (or whatever you decide is best).
procbuild/listener.py
Outdated
| msg = await self.socket.recv_multipart() | ||
| target, raw_payload = msg | ||
| payload = json.loads(raw_payload.decode('utf-8')) | ||
| print('received', payload) |
There was a problem hiding this comment.
Ok I'll remove it, I found it useful even when it was running to know that it was receiving things appropriately and it would show up in the heroku logs (I think). But I'm ok abandoning this.
procbuild/listener.py
Outdated
| in_queue = True | ||
| return in_queue | ||
|
|
||
| def check_age_and_queue(self, nr): |
There was a problem hiding this comment.
I'd recommend removing this method and use these two functions inline above. It doesn't add enough to warrant its own function.
| # await an item from the queue | ||
| nr = await self.queue.get() | ||
| # launch subprocess to build item | ||
| with ThreadPoolExecutor(max_workers=1) as e: |
There was a problem hiding this comment.
How many CPUs do we have on Heroku? We could ramp this up, if multiple are available.
There was a problem hiding this comment.
When i just ran heroku run grep -c processor /proc/cpuinfo I got 8 back… but I don't know if that will use up our dyno hours more quickly
|
Note the test failure does not actually mean anything since we have nothing setup with travis… |
mpacer
force-pushed
the
zmq_bus_build_manager
branch
from
a2a7d38
to
55dfb1e
Compare
procbuild/listener.py
Outdated
| main context for the listener class | ||
| socket: zmq.socket | ||
| the socket for listening to | ||
| queue: asyncio.Queue |
There was a problem hiding this comment.
I'm not sure how forgiving numpydoc is, but you might need spaces after the parameter name.
procbuild/submitter.py
Outdated
| @@ -7,21 +7,20 @@ | |||
| class BuildRequestSubmitter: | |||
| """Class for submitting build requests to zmq socket. | |||
|
|
|||
| Parameters: | |||
mpacer
force-pushed
the
zmq_bus_build_manager
branch
3 times, most recently
from
d6a7ae8
to
3005978
Compare
remove unused imports
procbuild/listener.py
Outdated
| """ | ||
| age = file_age(status_file(nr)) | ||
| min_wait = 0.5 | ||
| too_young = False |
There was a problem hiding this comment.
How about:
too_young = (age is not None) and (age <= min_wait)
if too_young:
log(f"...")
47f75fb
to
eb2db70
Compare
also make the code pattern nicer
mpacer
force-pushed
the
zmq_bus_build_manager
branch
from
eb2db70
to
c08b4d0
Compare
If #28 is merged this supercedes #25
This takes the works pursued in #25 and builds off of the work in #28 to have a build manager class that more directly exposes the structure of its subprocesses.