result_backend setting supports rediss:// protocol URLs

[remeika]

Implements support to configure a Redis + TLS connection to the result backend purely by using result_backend, rather than the setting redis_backend_use_ssl.

The fields within redis_backend_use_ssl are now configurable via URL parameters. This has two significant advantages:

1. A settings file does not need to import the ssl package in order to declare redis_backend_use_ssl.cert_reqs. This allows more configuration to be pulled from the environment rather than committed as code.
2. Projects that depend on celery do not need to expose an analogous setting to redis_backend_use_ssl; users can connect to Redis + TLS in dependent projects without any changes by maintainers.

Fixes #4695

[remeika]

@thedrow Done. Thanks, I had overlooked that part of the contribution docs.

[codecov]

Codecov Report

Merging #4696 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4696      +/-   ##
==========================================
+ Coverage    82.6%   82.62%   +0.02%     
==========================================
  Files         140      140              
  Lines       15868    15892      +24     
  Branches     1983     1989       +6     
==========================================
+ Hits        13107    13131      +24     
  Misses       2567     2567              
  Partials      194      194

Impacted Files	Coverage Δ
celery/app/backends.py	59.37% <ø> (ø)	⬆️
celery/backends/redis.py	91.25% <100%> (+0.87%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8c753d7...9c14f88. Read the comment docs.

[remeika]

Wow, I feel silly. Just added you as a collaborator so you can push.

…

On May 3, 2018, at 6:02 AM, Omer Katz ***@***.***> wrote: @thedrow requested changes on this pull request. I tried to fix your lint issues but I can't push to your branch. You have two tests with the same name. One of them is failing. I've grouped them into one test with a parameter. Here's the diff: diff --git a/t/unit/backends/test_redis.py b/t/unit/backends/test_redis.py index 33d30b81a..ca91af93b 100644 --- a/t/unit/backends/test_redis.py +++ b/t/unit/backends/test_redis.py @@ -326,20 +326,15 @@ class test_RedisBackend: from redis.connection import SSLConnection assert x.connparams['connection_class'] is SSLConnection - - @skip.unless_module('redis') - def test_backend_ssl_url_invalid(self): - with pytest.raises(ValueError): - x = self.Backend( - ***@***.***:123//1?ssl_cert_reqs=CERT_KITTY_CATS', - app=self.app, - ) - @skip.unless_module('redis') - def test_backend_ssl_url_invalid(self): + @pytest.mark.parametrize("uri", [ + ***@***.***:123//1?ssl_cert_reqs=CERT_KITTY_CATS', + 'rediss: ***@***.***:123//1' + ]) + def test_backend_ssl_url_invalid(self, uri): with pytest.raises(ValueError): - x = self.Backend( - ***@***.***:123//1', + self.Backend( + uri, app=self.app, ) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4696 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEufwoHLzROEO2ZuPMKaVdeu4B_EC6pCks5tutXMgaJpZM4Tti41>.

[archit]

How does one use this? I'm trying to use this in my Flask app, and I get this error ...

  File "/usr/local/lib/python3.6/site-packages/flask/app.py", line 1612, in full_dispatch_request
    rv = self.dispatch_request()
  File "/usr/local/lib/python3.6/site-packages/flask/app.py", line 1598, in dispatch_request
    return self.view_functions[rule.endpoint](**req.view_args)
  File "./disruptor/routes.py", line 684, in celery_dummy
    answer.delay()
  File "/usr/local/lib/python3.6/site-packages/celery/app/task.py", line 408, in delay
    return self.apply_async(args, kwargs)
  File "/usr/local/lib/python3.6/site-packages/celery/app/task.py", line 535, in apply_async
    **options
  File "/usr/local/lib/python3.6/site-packages/celery/app/base.py", line 741, in send_task
    with self.producer_or_acquire(producer) as P:
  File "/usr/local/lib/python3.6/site-packages/celery/app/base.py", line 876, in producer_or_acquire
    producer, self.producer_pool.acquire, block=True,
  File "/usr/local/lib/python3.6/site-packages/celery/app/base.py", line 1246, in producer_pool
    return self.amqp.producer_pool
  File "/usr/local/lib/python3.6/site-packages/celery/app/amqp.py", line 612, in producer_pool
    self.app.connection_for_write()]
  File "/usr/local/lib/python3.6/site-packages/celery/app/base.py", line 773, in connection_for_write
    return self._connection(url or self.conf.broker_write_url, **kwargs)
  File "/usr/local/lib/python3.6/site-packages/celery/app/base.py", line 841, in _connection
    'broker_connection_timeout', connect_timeout
  File "/usr/local/lib/python3.6/site-packages/kombu/connection.py", line 179, in __init__
    if not get_transport_cls(transport).can_parse_url:
  File "/usr/local/lib/python3.6/site-packages/kombu/transport/__init__.py", line 83, in get_transport_cls
    _transport_cache[transport] = resolve_transport(transport)
  File "/usr/local/lib/python3.6/site-packages/kombu/transport/__init__.py", line 63, in resolve_transport
    transport, alt))
KeyError: 'No such transport: rediss.  Did you mean redis?'

Seems like there is some integration issue between this PR within celery and kombu.

[remeika]

@archit Please post the results of the following commands:

pip freeze | grep celery
pip freeze | grep kombu

[archit]

@remeika here's the output from my setup...

[archit@ip-10-0-2-54 ~]$  sudo docker exec 3cb pip freeze | grep celery
celery==4.2.0
[archit@ip-10-0-2-54 ~]$  sudo docker exec 3cb pip freeze | grep kombu
kombu==4.2.1
[archit@ip-10-0-2-54 ~]$

[remeika]

Thanks for that. Are you trying to use Redis TLS as a broker or a results backend? Sadly this PR only covers the latter.

…

On Jun 18, 2018, at 5:17 PM, Archit Baweja ***@***.***> wrote: @remeika here's the output from my setup... ***@***.*** ~]$ sudo docker exec 3cb pip freeze | grep celery celery==4.2.0 ***@***.*** ~]$ sudo docker exec 3cb pip freeze | grep kombu kombu==4.2.1 ***@***.*** ~]$ — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[georgepsarakis]

@archit @remeika I believe the broker_use_ssl setting is the equivalent.

[archit]

@remeika @georgepsarakis oh ok, that makes sense. I'm using redis as a broker. I did not realize this was not for the broker. Thanks for clarifying.

[remeika]

@archit See here for why I added this feature to one but not the other.

Issue to add support for rediss:// URL protocol to broker_url is #4694. I am going to try to find time to work on it soon. But if you beat me to it, I'd be overjoyed 😃.

[archit]

@remeika haha, sure thing. Let me see if I can make some time. I will have to review some of the details in CONTRIBUTING.rst. Also I first have to get to make this work using broker_use_ssl, in my case.