Refactor to use concurrent.futures #12

aronasorman · 2017-05-24T20:05:29Z

Fixes #5 and #11. Changes:

refactor worker module from spawning our own worker threads, to using concurrent.futures.
simplify the worker implementation by removing the jobqueue and reportqueue. jobqueue is now handled by concurrent.futures, and then we just send any job updates straight to the messaging backend, removing the need for the reportqueue.
fix tests. Now only ~~1 test is failing~~ all tests are passing on python 3.6!

Added:

Now implements progress tracking too.
Implements error reporting too.

So the general API found in InMemClient found here should be ready to use in Kolibri.

Added (05/26/2017):

simplified Scheduler implementation
added docstrings for Client (the main API) and Job
Add changelog !1875cab

…ility to the Job class.

…orever.

…a Job

Now sleep even through success or failure 💤

…gress updating.

Or else the state we have for the job will be outdated.

…own thread instance.

aronasorman · 2017-05-26T20:17:03Z

@benjaoming you'll be happy to know that I added a changelog based on http://keepachangelog.com/en/0.3.0/ :)

CHANGELOG.md

+All notable changes to this project will be documented in this file.
+
+The format is based on [Keep a Changelog](http://keepachangelog.com/)
+and this project adheres to [Semantic Versioning](http://semver.org/).


CHANGELOG.md

+and this project adheres to [Semantic Versioning](http://semver.org/).
+
+
+## [Unreleased]


setup.py


 def read_file(fname):
    """
    Read file and decode in py2k
    """
-    if sys.version_info < (3,):
+    if IS_PYTHON_2:


src/barbequeue/common/classes.py

+
+    Jobs are stored on the storage backend for persistence through restarts, and are scheduled for running
+    to the workers.
+    """
    class State(enum.Enum):


src/barbequeue/common/classes.py

-    def shutdown(self):
-        # stub method, override if you need a more complex shut down procedure.
-        pass
+        return float(self.progress) / self.total_progress


src/barbequeue/scheduler/classes.py

-        self.scheduler_thread.setDaemon(True)
+    def start_scheduler(self):
+        self.scheduler_thread = InfiniteLoopThread(func=self.schedule_next_job, thread_name="SCHEDULER",
+                                                   wait_between_runs=0.5)


src/barbequeue/scheduler/classes.py

+    def start_worker_message_handler(self):
+        self.worker_message_handler_thread = InfiniteLoopThread(func=lambda: self.handle_worker_messages(timeout=2),
+                                                                thread_name="WORKERMESSAGEHANDLER",
+                                                                wait_between_runs=0.5)


src/barbequeue/storage/backends/default.py

+from collections import defaultdict
+from threading import Event
+
+JOB_EVENT_MAPPING = defaultdict(lambda: Event())


tests/common/test_classes.py

-        except queue.Empty:
-            # Maybe it's been processed already... just continue anyway then.
+            inmem_client._storage.wait_for_job_update(job_id, timeout=2)
+        except Exception:


tests/storage/backends/test_inmem.py

@@ -28,7 +29,7 @@ def test_can_schedule_single_job(self, defaultbackend, simplejob):
        new_job = defaultbackend.get_job(job_id)

        # Does the returned job record the function we set to run?
-        assert str(new_job.func) == str(id)
+        assert str(new_job.func) == stringify_func(id)


benjaoming · 2017-05-30T18:06:43Z

If you rebase/merge develop, we should have more accurate test failures regarding Python 2.7 and pypy.

aronasorman · 2017-05-31T23:29:04Z

Thanks! Looking over your changes now. Some quick notes:

The usage of the compat.py file seems to be a bit weird for me. I'd rather import six and use the compatibility modules there, as I'm imagining that we'll need more of those in the future.

I generally don't put logging statements in my testing code, as the default logging.debug statements i sprinkled in already provide enough data to help debug (apart from running just one test that's failing). What do you think?

benjaoming · 2017-06-02T14:51:39Z

The usage of the compat.py file seems to be a bit weird for me. I'd rather import six and use the compatibility modules there, as I'm imagining that we'll need more of those in the future.

If six does it, we should use six. For things that aren't supplied in six, it's very important to gather the compatibility hacks in one place. Otherwise they linger around forever. Been removing plenty of stale 2.5 and 2.6 compatibility hacks :)

benjaoming · 2017-06-02T15:04:38Z

I generally don't put logging statements in my testing code, as the default logging.debug statements i sprinkled in already provide enough data to help debug (apart from running just one test that's failing). What do you think?

We can do lots of logging because pytest is clever enough to not display any of the logging done by successful tests.. Anyways, having lots of logging will also make test code less readable. So probably not so nice to leave it around. Was looking at test code in Django, and they don't log anything.

I don't remember where my original comment about it was, but it had to do with a test that froze and I couldn't even know what test it was executing and get the logging output. So in this case I could troubleshoot it by logging and using pytest -s so it would forward the logging directly to stderr.

Jobs aren't being viewed still :(

benjaoming · 2017-06-08T21:42:15Z

Test builds are complaining about not finding a barbequeue distribution, did you start the name change?

latter is not an exception

Else we have contentions for connections

Needed to have feature parity with django_q.

…ype and storage location

benjaoming · 2017-06-14T20:21:35Z

@aronasorman let me know anytime you want a mid-term review :)

benjaoming · 2017-06-23T14:59:00Z

Hi @aronasorman !

So the commit 391bb9f in this unmerged PR is the one that Kolibri is currently running on? :)

There are two quick comments remaining from my previous review, that I think you can fix up quickly?

I'm optimistic we should get this merged and try to get on track with smaller issues, PRs, and versioning+releasing of Iceqube (yeah the name refactor is probably best to have cleared, too).. it seems easier than sitting on this rather lengthy PR and starting to release Kolibri with some commit ID ref'ed in a dependency link.

Apparently on Android, even just importing processpoolexecutor would cause an error.

aronasorman · 2017-06-26T21:24:20Z

Think I addressed the important comments, merging this in now!

requirements.txt

 pytest-mock==1.6.0
-pytest-cov


aronasorman added 7 commits May 23, 2017 16:01

Move function fully qualified name saving from the Client's responsib…

cd1b76d

…ility to the Job class.

Add SuccessMessage type

a9fb3fa

Simplify Worker backend by using concurrent.futures.

4129774

Implement InfiniteLoopThread, a generic thread that runs a function f…

72cbc9a

…orever.

Add timeout parameter to messaging.pop abstractmethod

d1f9e40

fix job completion marking

b6c8a62

flesh out worker sending of success message.

07d5f5c

aronasorman requested a review from benjaoming May 24, 2017 20:06

aronasorman added 18 commits May 24, 2017 17:48

Add the queued job state, and give a better string representation of …

f332128

…a Job

fix bug InfiniteLoopThread only sleeps when we get an exception.

a16ff34

Now sleep even through success or failure 💤

properly mark jobs as running, then take them off the queue.

8c53e89

catch a possible timeout exception from waiting for a job update.

d3e9a86

Extend get_lambda_to_execute to support passing in a function for pro…

881679b

…gress updating.

Create a ProgressMessage class for updating our progress.

77a2b53

Create a new FailureMessage class

b440efa

Allow job progress updates.

a0fe416

Fetch the job AFTER we get an update about it.

7505575

Or else the state we have for the job will be outdated.

Implement failure notification for jobs.

e343908

rename InfiniteLoopThread.cancel to stop

5b69f13

refactor Scheduler to use InfiniteLoopThread instead of defining its …

8b06b90

…own thread instance.

add docstrings for the client classes

2fbdaab

remove unused functions and classes

d4bcf4a

add comment documentation for the job's states

31c5280

Properly set the initial values of traceback and exception

920a380

add docstrings for Job.

664bab4

Add initial changelog.

1875cab

benjaoming suggested changes May 29, 2017

View reviewed changes

Merge remote-tracking branch 'origin/develop' into refactor-futures

96d7d8e

aronasorman added 3 commits June 5, 2017 11:40

get tests passing on python27

129c2eb

Add sqlalchemy and remove enum34

f4b5cdf

truly remove enum dependency

24602c5

aronasorman force-pushed the refactor-futures branch from ca768f4 to 24602c5 Compare June 7, 2017 20:59

second pass on a SQL-based job storage backend.

a851417

Jobs aren't being viewed still :(

aronasorman added 11 commits June 9, 2017 15:32

implement barbequeue.exceptions.TimeoutError

2462622

Fix test that expects MessageType to be an enum

8635de4

use NotImplementedError, not NotImplemented

ec8f693

latter is not an exception

make sure read sessions are closed.

1b49bb2

Else we have contentions for connections

more fixes to get stuff working on sqlalchemy

d02a858

add client.all_jobs() and client.clear()

e92205c

Needed to have feature parity with django_q.

Properly set orm_job.obj value.

92db474

allow failed jobs to track the exception and traceback

90dca67

Fix clearing of jobs.

8f74ba8

Implement SimpleClient to allow users to specify the worker process t…

be50717

…ype and storage location

remove pdb call from simpletest

1554030

aronasorman added 2 commits June 21, 2017 11:11

Always have concurrent-futures as a dependency.

391bb9f

Add wait and wait_for_completion on the client class

b2687fa

conditionally import processpool and threadpool.

92b020b

Apparently on Android, even just importing processpoolexecutor would cause an error.

aronasorman merged commit 95212eb into develop Jun 26, 2017

benjaoming reviewed Jun 28, 2017

View reviewed changes

requirements.txt

pytest-mock==1.6.0

pytest-cov

This comment was marked as spam.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor to use concurrent.futures #12

Refactor to use concurrent.futures #12

aronasorman commented May 24, 2017 •

edited

Loading

aronasorman commented May 26, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

benjaoming commented May 30, 2017 •

edited

Loading

aronasorman commented May 31, 2017

benjaoming commented Jun 2, 2017

benjaoming commented Jun 2, 2017

benjaoming commented Jun 8, 2017

benjaoming commented Jun 14, 2017

benjaoming commented Jun 23, 2017

aronasorman commented Jun 26, 2017

This comment was marked as spam.

		and this project adheres to [Semantic Versioning](http://semver.org/).


		## [Unreleased]

Refactor to use concurrent.futures #12

Refactor to use concurrent.futures #12

Conversation

aronasorman commented May 24, 2017 • edited Loading

aronasorman commented May 26, 2017

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

benjaoming commented May 30, 2017 • edited Loading

aronasorman commented May 31, 2017

benjaoming commented Jun 2, 2017

benjaoming commented Jun 2, 2017

benjaoming commented Jun 8, 2017

benjaoming commented Jun 14, 2017

benjaoming commented Jun 23, 2017

aronasorman commented Jun 26, 2017

This comment was marked as spam.

aronasorman commented May 24, 2017 •

edited

Loading

benjaoming commented May 30, 2017 •

edited

Loading