From 9fcc0db9ace9887ee59013f191e5e5b5bc17722f Mon Sep 17 00:00:00 2001 From: Scott Dillon Date: Fri, 21 Dec 2018 10:00:58 -0500 Subject: [PATCH 01/15] [#3994] Pass a timeout through to the rq.Job object. Pass the job timeout kwarg through to the `rq.queue.enqueue_call` method. Get the default job timeout from the ckan config. --- ckan/lib/jobs.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ckan/lib/jobs.py b/ckan/lib/jobs.py index 692f2aac2a6..b5438eee60d 100644 --- a/ckan/lib/jobs.py +++ b/ckan/lib/jobs.py @@ -36,6 +36,7 @@ log = logging.getLogger(__name__) DEFAULT_QUEUE_NAME = u'default' +DEFAULT_JOB_TIMEOUT = config[u'rq.bg_job_timeout'] # RQ job queues. Do not use this directly, use ``get_queue`` instead. _queues = {} @@ -123,7 +124,8 @@ def get_queue(name=DEFAULT_QUEUE_NAME): return queue -def enqueue(fn, args=None, kwargs=None, title=None, queue=DEFAULT_QUEUE_NAME): +def enqueue(fn, args=None, kwargs=None, title=None, queue=DEFAULT_QUEUE_NAME, + timeout=DEFAULT_JOB_TIMEOUT): u''' Enqueue a job to be run in the background. @@ -141,6 +143,9 @@ def enqueue(fn, args=None, kwargs=None, title=None, queue=DEFAULT_QUEUE_NAME): :param string queue: Name of the queue. If not given then the default queue is used. + :param integer timeout: The timeout, in seconds, to be passed + to the background job via rq. + :returns: The enqueued job. :rtype: ``rq.job.Job`` ''' @@ -148,7 +153,8 @@ def enqueue(fn, args=None, kwargs=None, title=None, queue=DEFAULT_QUEUE_NAME): args = [] if kwargs is None: kwargs = {} - job = get_queue(queue).enqueue_call(func=fn, args=args, kwargs=kwargs) + job = get_queue(queue).enqueue_call(func=fn, args=args, kwargs=kwargs, + timeout=None) job.meta[u'title'] = title job.save() msg = u'Added background job {}'.format(job.id) From a0cfa22b52e1d232df926952ba508da3ad398734 Mon Sep 17 00:00:00 2001 From: Scott Dillon Date: Fri, 21 Dec 2018 10:01:45 -0500 Subject: [PATCH 02/15] [#3994] Add default backound job timeout to deployment.ini template. --- ckan/config/deployment.ini_tmpl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ckan/config/deployment.ini_tmpl b/ckan/config/deployment.ini_tmpl index aaa3bf26f9a..182cfe984e0 100644 --- a/ckan/config/deployment.ini_tmpl +++ b/ckan/config/deployment.ini_tmpl @@ -226,3 +226,6 @@ formatter = generic [formatter_generic] format = %(asctime)s %(levelname)-5.5s [%(name)s] %(message)s + +## RQ Job Settings +rq.bg_job_timeout = 180 From 5d8935fa5c076087c20b0ed9b1fcb999e1b23d9a Mon Sep 17 00:00:00 2001 From: Scott Dillon Date: Fri, 21 Dec 2018 10:04:46 -0500 Subject: [PATCH 03/15] [#3994] Test for accurate timeout values in the rq Job. Test that a timeout was passed to the rq Job objects when called from `ckan.lib.jobs.py`. Test if timeout is not passed and values of 0, -1 and 3600 are passed correctly. --- ckan/tests/lib/test_jobs.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ckan/tests/lib/test_jobs.py b/ckan/tests/lib/test_jobs.py index 7cdfc80bc3c..3d8dbcb05d2 100644 --- a/ckan/tests/lib/test_jobs.py +++ b/ckan/tests/lib/test_jobs.py @@ -74,6 +74,18 @@ def test_enqueue_queue(self): assert_equal(all_jobs[1].origin, jobs.add_queue_name_prefix(u'my_queue')) + def test_enqueue_timeout(self): + self.enqueue() + self.enqueue(timeout=0) + self.enqueue(timeout=-1) + self.enqueue(timeout=3600) + all_jobs = self.all_jobs() + assert_equal(len(all_jobs), 4) + assert_equal(all_jobs[0].meta[u'timeout'], 180) + assert_equal(all_jobs[1].meta[u'timeout'], 180) + assert_equal(all_jobs[2].meta[u'timeout'], -1) + assert_equal(all_jobs[3].meta[u'timeout'], 3600) + class TestGetAllQueues(RQTestBase): From 907c60dca8c196e4d025b6772110ffb9e80f5686 Mon Sep 17 00:00:00 2001 From: Scott Dillon Date: Fri, 21 Dec 2018 10:37:51 -0500 Subject: [PATCH 04/15] [#3994] Background job timeout documentation Detail how to add a custom timeout to a background job and the location of default value in the ckan config file. --- doc/maintaining/background-tasks.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/maintaining/background-tasks.rst b/doc/maintaining/background-tasks.rst index 002e645e04b..172f67439d6 100644 --- a/doc/maintaining/background-tasks.rst +++ b/doc/maintaining/background-tasks.rst @@ -99,6 +99,12 @@ You can also give the job a title which can be useful for identifying it when jobs.enqueue(log_job, [u'My log message'], title=u'My log job') +A timeout can also be set on a job iwth the ``timeout`` keyword argument:: + + jobs.enqueue(log_job, [u'My log message'], timeout=3600) + +The default background job timeout is 180 seconds. This is set in the +ckan config ``.ini`` file under the ``rq.bg_job_timeout`` item. Accessing the database from background jobs ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From f3f841f84a3417259fa14610e8e32e4dea3fb6c0 Mon Sep 17 00:00:00 2001 From: Scott Dillon Date: Fri, 21 Dec 2018 10:53:47 -0500 Subject: [PATCH 05/15] [#3994] Add job timeout to circle CI settings file. --- test-core-circle-ci.ini | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test-core-circle-ci.ini b/test-core-circle-ci.ini index a558853e415..5b0680b3967 100644 --- a/test-core-circle-ci.ini +++ b/test-core-circle-ci.ini @@ -47,3 +47,6 @@ formatter = generic [formatter_generic] format = %(asctime)s %(levelname)-5.5s [%(name)s] %(message)s + +[rq] +rq.bg_job_timeout = 180 From 5b83e01fd8d9a6220fefe1a434370b8f206d998a Mon Sep 17 00:00:00 2001 From: Scott Dillon Date: Fri, 21 Dec 2018 11:11:12 -0500 Subject: [PATCH 06/15] [#3994] Move timeout setting under the `[app:main]` header in ini file. --- ckan/config/deployment.ini_tmpl | 5 ++--- test-core-circle-ci.ini | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/ckan/config/deployment.ini_tmpl b/ckan/config/deployment.ini_tmpl index 182cfe984e0..30d81722930 100644 --- a/ckan/config/deployment.ini_tmpl +++ b/ckan/config/deployment.ini_tmpl @@ -191,6 +191,8 @@ ckan.hide_activity_from_users = %(ckan.site_id)s #smtp.password = your_password #smtp.mail_from = +## RQ Job Settings +rq.bg_job_timeout = 180 ## Logging configuration [loggers] @@ -226,6 +228,3 @@ formatter = generic [formatter_generic] format = %(asctime)s %(levelname)-5.5s [%(name)s] %(message)s - -## RQ Job Settings -rq.bg_job_timeout = 180 diff --git a/test-core-circle-ci.ini b/test-core-circle-ci.ini index 5b0680b3967..0a5cc261dda 100644 --- a/test-core-circle-ci.ini +++ b/test-core-circle-ci.ini @@ -13,6 +13,8 @@ sqlalchemy.url = postgresql://ckan_default:pass@ckan-postgres/ckan_test solr_url = http://localhost:8080/solr +rq.bg_job_timeout = 180 + [loggers] keys = root, ckan, sqlalchemy @@ -47,6 +49,3 @@ formatter = generic [formatter_generic] format = %(asctime)s %(levelname)-5.5s [%(name)s] %(message)s - -[rq] -rq.bg_job_timeout = 180 From 76d97865a6150d5714c5b0e387e23ff090cbe16f Mon Sep 17 00:00:00 2001 From: Scott Dillon Date: Fri, 21 Dec 2018 14:53:12 -0500 Subject: [PATCH 07/15] [#3994] Move backgroud job timeout to `test-core.ini` --- test-core-circle-ci.ini | 2 -- test-core.ini | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/test-core-circle-ci.ini b/test-core-circle-ci.ini index 0a5cc261dda..a558853e415 100644 --- a/test-core-circle-ci.ini +++ b/test-core-circle-ci.ini @@ -13,8 +13,6 @@ sqlalchemy.url = postgresql://ckan_default:pass@ckan-postgres/ckan_test solr_url = http://localhost:8080/solr -rq.bg_job_timeout = 180 - [loggers] keys = root, ckan, sqlalchemy diff --git a/test-core.ini b/test-core.ini index 879ee5547b3..e0d078eb58e 100644 --- a/test-core.ini +++ b/test-core.ini @@ -101,6 +101,9 @@ who.config_file = %(here)s/who.ini who.log_level = warning who.log_file = %(cache_dir)s/who_log.ini +## rq background jobs +rq.bg_job_timeout = 180 + # Logging configuration [loggers] keys = root, ckan, sqlalchemy From dc7532d6ecd6c27de188a638c9e3aec3ec8d96f9 Mon Sep 17 00:00:00 2001 From: Scott Dillon Date: Tue, 8 Jan 2019 10:16:26 -0500 Subject: [PATCH 08/15] [#3994] Perform a lookup in config dict to get a default value. --- ckan/lib/jobs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/lib/jobs.py b/ckan/lib/jobs.py index b5438eee60d..588dee416d7 100644 --- a/ckan/lib/jobs.py +++ b/ckan/lib/jobs.py @@ -36,7 +36,7 @@ log = logging.getLogger(__name__) DEFAULT_QUEUE_NAME = u'default' -DEFAULT_JOB_TIMEOUT = config[u'rq.bg_job_timeout'] +DEFAULT_JOB_TIMEOUT = config.get[u'rq.bg_job_timeout', 180] # RQ job queues. Do not use this directly, use ``get_queue`` instead. _queues = {} From 28d78d3cc912e6d38e9505eabb753c01dfdfa5b4 Mon Sep 17 00:00:00 2001 From: Scott Dillon Date: Tue, 8 Jan 2019 10:29:00 -0500 Subject: [PATCH 09/15] [#3994] Fix bracket error. --- ckan/lib/jobs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/lib/jobs.py b/ckan/lib/jobs.py index 588dee416d7..ca65fe964c1 100644 --- a/ckan/lib/jobs.py +++ b/ckan/lib/jobs.py @@ -36,7 +36,7 @@ log = logging.getLogger(__name__) DEFAULT_QUEUE_NAME = u'default' -DEFAULT_JOB_TIMEOUT = config.get[u'rq.bg_job_timeout', 180] +DEFAULT_JOB_TIMEOUT = config.get(u'rq.bg_job_timeout', 180) # RQ job queues. Do not use this directly, use ``get_queue`` instead. _queues = {} From 897082f04eed02e39d756c5c1b62ef149808b3e4 Mon Sep 17 00:00:00 2001 From: Scott Dillon Date: Tue, 8 Jan 2019 11:15:39 -0500 Subject: [PATCH 10/15] [#3994] Change timeout access to attribute from meta dict lookup. --- ckan/tests/lib/test_jobs.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ckan/tests/lib/test_jobs.py b/ckan/tests/lib/test_jobs.py index 3d8dbcb05d2..d2a1d279d97 100644 --- a/ckan/tests/lib/test_jobs.py +++ b/ckan/tests/lib/test_jobs.py @@ -81,10 +81,10 @@ def test_enqueue_timeout(self): self.enqueue(timeout=3600) all_jobs = self.all_jobs() assert_equal(len(all_jobs), 4) - assert_equal(all_jobs[0].meta[u'timeout'], 180) - assert_equal(all_jobs[1].meta[u'timeout'], 180) - assert_equal(all_jobs[2].meta[u'timeout'], -1) - assert_equal(all_jobs[3].meta[u'timeout'], 3600) + assert_equal(all_jobs[0].timeout, 180) + assert_equal(all_jobs[1].timeout, 180) + assert_equal(all_jobs[2].timeout, -1) + assert_equal(all_jobs[3].timeout, 3600) class TestGetAllQueues(RQTestBase): From b81155230dbca991c87d2c21f62357803441c0fb Mon Sep 17 00:00:00 2001 From: Scott Dillon Date: Tue, 8 Jan 2019 11:24:53 -0500 Subject: [PATCH 11/15] [#3994] Fix test reference value when setting timeout to -1. redis queue sets the timeout to the default instead of using no timeout. --- ckan/tests/lib/test_jobs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/tests/lib/test_jobs.py b/ckan/tests/lib/test_jobs.py index d2a1d279d97..34694ca2d14 100644 --- a/ckan/tests/lib/test_jobs.py +++ b/ckan/tests/lib/test_jobs.py @@ -83,7 +83,7 @@ def test_enqueue_timeout(self): assert_equal(len(all_jobs), 4) assert_equal(all_jobs[0].timeout, 180) assert_equal(all_jobs[1].timeout, 180) - assert_equal(all_jobs[2].timeout, -1) + assert_equal(all_jobs[2].timeout, 180) assert_equal(all_jobs[3].timeout, 3600) From 2ed66c47ebb786e66e94a5d551957f0b8bb3699e Mon Sep 17 00:00:00 2001 From: Scott Dillon Date: Tue, 8 Jan 2019 13:46:45 -0500 Subject: [PATCH 12/15] [#3994] Pass the timeout to the function instead of None --- ckan/lib/jobs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/lib/jobs.py b/ckan/lib/jobs.py index ca65fe964c1..196b241c111 100644 --- a/ckan/lib/jobs.py +++ b/ckan/lib/jobs.py @@ -154,7 +154,7 @@ def enqueue(fn, args=None, kwargs=None, title=None, queue=DEFAULT_QUEUE_NAME, if kwargs is None: kwargs = {} job = get_queue(queue).enqueue_call(func=fn, args=args, kwargs=kwargs, - timeout=None) + timeout=timeout) job.meta[u'title'] = title job.save() msg = u'Added background job {}'.format(job.id) From abef638041acdc880f0ec45bcbe95de6eb80e311 Mon Sep 17 00:00:00 2001 From: Scott Dillon Date: Tue, 8 Jan 2019 13:54:03 -0500 Subject: [PATCH 13/15] [#3994] Fix the test reference value since it does support -1 as a timeout value. --- ckan/tests/lib/test_jobs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ckan/tests/lib/test_jobs.py b/ckan/tests/lib/test_jobs.py index 34694ca2d14..d2a1d279d97 100644 --- a/ckan/tests/lib/test_jobs.py +++ b/ckan/tests/lib/test_jobs.py @@ -83,7 +83,7 @@ def test_enqueue_timeout(self): assert_equal(len(all_jobs), 4) assert_equal(all_jobs[0].timeout, 180) assert_equal(all_jobs[1].timeout, 180) - assert_equal(all_jobs[2].timeout, 180) + assert_equal(all_jobs[2].timeout, -1) assert_equal(all_jobs[3].timeout, 3600) From 85ee64bddae5c13921cf83e0a5d4dec7e862296c Mon Sep 17 00:00:00 2001 From: Scott Dillon Date: Tue, 15 Jan 2019 16:45:55 -0500 Subject: [PATCH 14/15] Change references to config variable to be implementation agnostic. --- ckan/config/deployment.ini_tmpl | 4 ++-- ckan/lib/jobs.py | 2 +- doc/maintaining/background-tasks.rst | 2 +- test-core.ini | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ckan/config/deployment.ini_tmpl b/ckan/config/deployment.ini_tmpl index 30d81722930..ae4af0a6756 100644 --- a/ckan/config/deployment.ini_tmpl +++ b/ckan/config/deployment.ini_tmpl @@ -191,8 +191,8 @@ ckan.hide_activity_from_users = %(ckan.site_id)s #smtp.password = your_password #smtp.mail_from = -## RQ Job Settings -rq.bg_job_timeout = 180 +## Background Job Settings +bg_job_timeout = 180 ## Logging configuration [loggers] diff --git a/ckan/lib/jobs.py b/ckan/lib/jobs.py index 196b241c111..2a52ceec3ea 100644 --- a/ckan/lib/jobs.py +++ b/ckan/lib/jobs.py @@ -36,7 +36,7 @@ log = logging.getLogger(__name__) DEFAULT_QUEUE_NAME = u'default' -DEFAULT_JOB_TIMEOUT = config.get(u'rq.bg_job_timeout', 180) +DEFAULT_JOB_TIMEOUT = config.get(u'bg_job_timeout', 180) # RQ job queues. Do not use this directly, use ``get_queue`` instead. _queues = {} diff --git a/doc/maintaining/background-tasks.rst b/doc/maintaining/background-tasks.rst index 172f67439d6..316d334ae6a 100644 --- a/doc/maintaining/background-tasks.rst +++ b/doc/maintaining/background-tasks.rst @@ -104,7 +104,7 @@ A timeout can also be set on a job iwth the ``timeout`` keyword argument:: jobs.enqueue(log_job, [u'My log message'], timeout=3600) The default background job timeout is 180 seconds. This is set in the -ckan config ``.ini`` file under the ``rq.bg_job_timeout`` item. +ckan config ``.ini`` file under the ``bg_job_timeout`` item. Accessing the database from background jobs ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/test-core.ini b/test-core.ini index e0d078eb58e..1a34586539f 100644 --- a/test-core.ini +++ b/test-core.ini @@ -101,8 +101,8 @@ who.config_file = %(here)s/who.ini who.log_level = warning who.log_file = %(cache_dir)s/who_log.ini -## rq background jobs -rq.bg_job_timeout = 180 +## background jobs +bg_job_timeout = 180 # Logging configuration [loggers] From 6382d1ec158c6724270e67a6aaf7f51edf56725e Mon Sep 17 00:00:00 2001 From: Scott Dillon Date: Thu, 17 Jan 2019 10:18:01 -0500 Subject: [PATCH 15/15] [#3994] Scope the background job timeout to ckan. --- ckan/config/deployment.ini_tmpl | 2 +- ckan/lib/jobs.py | 2 +- doc/maintaining/background-tasks.rst | 2 +- test-core.ini | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ckan/config/deployment.ini_tmpl b/ckan/config/deployment.ini_tmpl index ae4af0a6756..741c9f89a91 100644 --- a/ckan/config/deployment.ini_tmpl +++ b/ckan/config/deployment.ini_tmpl @@ -192,7 +192,7 @@ ckan.hide_activity_from_users = %(ckan.site_id)s #smtp.mail_from = ## Background Job Settings -bg_job_timeout = 180 +ckan.jobs.timeout = 180 ## Logging configuration [loggers] diff --git a/ckan/lib/jobs.py b/ckan/lib/jobs.py index 2a52ceec3ea..500b8fabf12 100644 --- a/ckan/lib/jobs.py +++ b/ckan/lib/jobs.py @@ -36,7 +36,7 @@ log = logging.getLogger(__name__) DEFAULT_QUEUE_NAME = u'default' -DEFAULT_JOB_TIMEOUT = config.get(u'bg_job_timeout', 180) +DEFAULT_JOB_TIMEOUT = config.get(u'ckan.jobs.timeout', 180) # RQ job queues. Do not use this directly, use ``get_queue`` instead. _queues = {} diff --git a/doc/maintaining/background-tasks.rst b/doc/maintaining/background-tasks.rst index 316d334ae6a..7e2f83a2752 100644 --- a/doc/maintaining/background-tasks.rst +++ b/doc/maintaining/background-tasks.rst @@ -104,7 +104,7 @@ A timeout can also be set on a job iwth the ``timeout`` keyword argument:: jobs.enqueue(log_job, [u'My log message'], timeout=3600) The default background job timeout is 180 seconds. This is set in the -ckan config ``.ini`` file under the ``bg_job_timeout`` item. +ckan config ``.ini`` file under the ``ckan.jobs.timeout`` item. Accessing the database from background jobs ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/test-core.ini b/test-core.ini index 1a34586539f..febb0d82ca1 100644 --- a/test-core.ini +++ b/test-core.ini @@ -102,7 +102,7 @@ who.log_level = warning who.log_file = %(cache_dir)s/who_log.ini ## background jobs -bg_job_timeout = 180 +ckan.jobs.timeout = 180 # Logging configuration [loggers]