Switch status API to RQ #960

bmbouter · 2018-04-20T18:48:36Z

The attribute name in the status API changed when RQ with Redis replaced
AMQP brokers like RabbitMQ.

This PR is meant to be used with these core and pulp_file PRs:

This PR does the following: * port travis to RQ * remove all celery references * install, systemd docs updates * create a custom RQ worker method for Pulp's needs * ports the status API to report on the Redis connection * port orphan cleanup tasks to work with RQ * port all pulp tasks to RQ * handle fatal error exception recording on tasks * replace apply_async_with_reservation to enqueue_with_reservation * task cancellation kills a running task correctly * work discovery, normal shutdown, and crash shutdown tested * adds settings for Redis connection to settings files * removes celery as a dependency * adds rq as a dependency There is a devel repo PR here to be used by `vagrant up` when testing this PR: pulp/devel#146 Required PR: pulp/pulp-smash#960 Required PR: pulp/pulp_file#72

dralley · 2018-04-20T21:32:30Z

This PR should be renamed and the scope expanded a bit, I think. The pulp-smash settings create command probably should be modified, along with the settings file it outputs. There is no longer any need for the broker settings nor asking the user if they are using Qpid.

nixocio · 2018-04-23T13:05:50Z

@dralley , please, can you file an smash issue providing more details, and outlining the requested changes?

dralley · 2018-04-23T13:33:44Z

@kersommoura #965

This PR does the following: * port travis to RQ * remove all celery references * install, systemd docs updates * create a custom RQ worker method for Pulp's needs * ports the status API to report on the Redis connection * port orphan cleanup tasks to work with RQ * port all pulp tasks to RQ * handle fatal error exception recording on tasks * replace apply_async_with_reservation to enqueue_with_reservation * task cancellation kills a running task correctly * work discovery, normal shutdown, and crash shutdown tested * adds settings for Redis connection to settings files * removes celery as a dependency * adds rq as a dependency There is a devel repo PR here to be used by `vagrant up` when testing this PR: pulp/devel#146 Required PR: pulp/pulp-smash#960 Required PR: pulp/pulp_file#72

Required PR: pulp/pulp-smash#960 Required PR: pulp/pulp_file#72

This PR does the following: * port travis to RQ * remove all celery references * install, systemd docs updates * create a custom RQ worker method for Pulp's needs * ports the status API to report on the Redis connection * port orphan cleanup tasks to work with RQ * port all pulp tasks to RQ * handle fatal error exception recording on tasks * replace apply_async_with_reservation to enqueue_with_reservation * task cancellation kills a running task correctly * work discovery, normal shutdown, and crash shutdown tested * adds settings for Redis connection to settings files * removes celery as a dependency * adds rq as a dependency There is a devel repo PR here to be used by `vagrant up` when testing this PR: pulp/devel#146 Required PR: pulp/pulp-smash#960 Required PR: pulp/pulp_file#72

Required PR: pulp/pulp-smash#960 Required PR: pulp/pulp_file#72

This PR does the following: * port travis to RQ * remove all celery references * install, systemd docs updates * create a custom RQ worker method for Pulp's needs * ports the status API to report on the Redis connection * port orphan cleanup tasks to work with RQ * port all pulp tasks to RQ * handle fatal error exception recording on tasks * replace apply_async_with_reservation to enqueue_with_reservation * task cancellation kills a running task correctly * work discovery, normal shutdown, and crash shutdown tested * adds settings for Redis connection to settings files * removes celery as a dependency * adds rq as a dependency There is a devel repo PR here to be used by `vagrant up` when testing this PR: pulp/devel#146 Required PR: pulp/pulp-smash#960 Required PR: pulp/pulp_file#72

Ichimonji10 · 2018-04-24T14:21:24Z

As @dralley correctly points out, some changes to the config file and CLI are in order. The issue that was filed is helpful. In the mean time, this particular test case does need to be updated.

I've refactored the test case targeted by this PR, making this PR out of date. See 7f8daac. Sorry about that. I made the refactor before seeing this. Thankfully, this PR can still be quite simple. Here's all that this PR needs to consist of:

diff --git a/pulp_smash/tests/pulp3/pulpcore/api_v3/test_status.py b/pulp_smash/tests/pulp3/pulpcore/api_v3/test_status.py
index 9204e73..512336d 100644
--- a/pulp_smash/tests/pulp3/pulpcore/api_v3/test_status.py
+++ b/pulp_smash/tests/pulp3/pulpcore/api_v3/test_status.py
@@ -21,7 +21,7 @@ STATUS = {
             'type': 'object',
             'properties': {'connected': {'type': 'boolean'}},
         },
-        'messaging_connection': {
+        'redis_connection': {
             'type': 'object',
             'properties': {'connected': {'type': 'boolean'}},
         },
@@ -92,7 +92,7 @@ class StatusTestCase(unittest.TestCase, utils.SmokeTest):
         """
         validate(status, STATUS)
         self.assertTrue(status['database_connection']['connected'])
-        self.assertTrue(status['messaging_connection']['connected'])
+        self.assertTrue(status['redis_connection']['connected'])
         self.assertEqual(status['missing_workers'], [])
         self.assertNotEqual(status['online_workers'], [])
         self.assertNotEqual(status['versions'], [])

Ichimonji10

Deal with refactored test case.

bmbouter · 2018-04-24T14:36:18Z

No problem at all I can rebase. I'll do that soon. Thank you @Ichimonji10. I'm also considering relabeling it back to 'tasking_connection' or something like that which was a feedback point from another PR. The thinking is that RQ can support other backends (at some point for instance etcd is on their roadmap). I'll make that change with my rebase too.

This PR does the following: * port travis to RQ * remove all celery references * install, systemd docs updates * create a custom RQ worker method for Pulp's needs * ports the status API to report on the Redis connection * port orphan cleanup tasks to work with RQ * port all pulp tasks to RQ * handle fatal error exception recording on tasks * replace apply_async_with_reservation to enqueue_with_reservation * task cancellation kills a running task correctly * work discovery, normal shutdown, and crash shutdown tested * adds settings for Redis connection to settings files * removes celery as a dependency * adds rq as a dependency There is a devel repo PR here to be used by `vagrant up` when testing this PR: pulp/devel#146 Required PR: pulp/pulp-smash#960 Required PR: pulp/pulp_file#72

bmbouter · 2018-04-27T20:32:45Z

@Ichimonji10 I pushed a rebased change. What else needs to be done in this PR to make it ready for merging?

Ichimonji10

LGTM. I assume that the changes were tested against a modified Pulp application. When this is merged, make sure to merge the corresponding Pulp PRs at the same time, as this change breaks currently passing tests.

The attribute name in the status API changed when RQ with Redis replaced AMQP brokers like RabbitMQ. This PR is meant to be used with these core and pulp_file PRs: pulp/pulp#3454 pulp/pulp_file#72

This PR does the following: * port travis to RQ * remove all celery references * install, systemd docs updates * create a custom RQ worker method for Pulp's needs * ports the status API to report on the Redis connection * port orphan cleanup tasks to work with RQ * port all pulp tasks to RQ * handle fatal error exception recording on tasks * replace apply_async_with_reservation to enqueue_with_reservation * task cancellation kills a running task correctly * work discovery, normal shutdown, and crash shutdown tested * adds settings for Redis connection to settings files * removes celery as a dependency * adds rq as a dependency There is a devel repo PR here to be used by `vagrant up` when testing this PR: pulp/devel#146 Required PR: pulp/pulp-smash#960 Required PR: pulp/pulp_file#72

* Updates Travis to use RQ worker runners * Remove usage of Celery * port apply_async_with_reservation to enqueue_with_reservation Required PR: pulp/pulp-smash#960 Required PR: pulp/pulp#3454

bmbouter mentioned this pull request Apr 20, 2018

Switch to RQ pulp/pulp_file#72

Merged

bmbouter mentioned this pull request Apr 20, 2018

Replace Celery with RQ pulp/pulp#3454

Merged

bmbouter force-pushed the switch-to-rq branch from c442a21 to e4e7b1f Compare April 20, 2018 19:25

bmbouter added the Pulp Version: 3 label Apr 20, 2018

dralley mentioned this pull request Apr 23, 2018

Modify Pulp Smash for the Pulp 3 transition to RQ #965

Closed

bmbouter force-pushed the switch-to-rq branch from e4e7b1f to e0a11eb Compare April 23, 2018 16:53

bmbouter pushed a commit to bmbouter/pulp that referenced this pull request Apr 23, 2018

test basic repo sync with logging captured

1071267

Required PR: pulp/pulp-smash#960 Required PR: pulp/pulp_file#72

bmbouter pushed a commit to bmbouter/pulp that referenced this pull request Apr 23, 2018

test basic repo sync with logging captured

2e711e7

Required PR: pulp/pulp-smash#960 Required PR: pulp/pulp_file#72

Ichimonji10 suggested changes Apr 24, 2018

View reviewed changes

bmbouter closed this Apr 27, 2018

bmbouter force-pushed the switch-to-rq branch from e0a11eb to 082394d Compare April 27, 2018 20:02

bmbouter reopened this Apr 27, 2018

Ichimonji10 approved these changes Apr 27, 2018

View reviewed changes

Switch status API to RQ

a0e3b04

The attribute name in the status API changed when RQ with Redis replaced AMQP brokers like RabbitMQ. This PR is meant to be used with these core and pulp_file PRs: pulp/pulp#3454 pulp/pulp_file#72

bmbouter force-pushed the switch-to-rq branch from 32a250b to a0e3b04 Compare May 7, 2018 21:32

bmbouter merged commit ed2626a into pulp:master May 14, 2018

bmbouter deleted the switch-to-rq branch May 14, 2018 18:19

Switch status API to RQ #960

Switch status API to RQ #960

bmbouter commented Apr 20, 2018 •

edited

dralley commented Apr 20, 2018 •

edited

nixocio commented Apr 23, 2018

dralley commented Apr 23, 2018

Ichimonji10 commented Apr 24, 2018 •

edited

Ichimonji10 left a comment

bmbouter commented Apr 24, 2018

bmbouter commented Apr 27, 2018

Ichimonji10 left a comment

Switch status API to RQ #960

Switch status API to RQ #960

Conversation

bmbouter commented Apr 20, 2018 • edited

dralley commented Apr 20, 2018 • edited

nixocio commented Apr 23, 2018

dralley commented Apr 23, 2018

Ichimonji10 commented Apr 24, 2018 • edited

Ichimonji10 left a comment

Choose a reason for hiding this comment

bmbouter commented Apr 24, 2018

bmbouter commented Apr 27, 2018

Ichimonji10 left a comment

Choose a reason for hiding this comment

bmbouter commented Apr 20, 2018 •

edited

dralley commented Apr 20, 2018 •

edited

Ichimonji10 commented Apr 24, 2018 •

edited