Add `rediss` (Redis over SSL) protocol to `RedisDsn` #1911

mykolasolodukha · 2020-09-07T18:05:25Z

Change Summary

Added rediss to RedisDsn.allowed_schemes.

Related issue number

This PR closes #1877

Checklist

Unit tests for the changes exist
Tests pass on CI and coverage remains at 100%
Documentation reflects the changes where applicable
changes/1911-TrDex.md file added describing change
(see changes/README.md for details)

codecov · 2020-09-07T18:08:33Z

Codecov Report

Merging #1911 (2dbbe5b) into master (31bc243) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master     #1911   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           21        21           
  Lines         4093      4133   +40     
  Branches       823       832    +9     
=========================================
+ Hits          4093      4133   +40

Impacted Files	Coverage Δ
pydantic/networks.py	`100.00% <100.00%> (ø)`
pydantic/json.py	`100.00% <0.00%> (ø)`
pydantic/types.py	`100.00% <0.00%> (ø)`
pydantic/fields.py	`100.00% <0.00%> (ø)`
pydantic/schema.py	`100.00% <0.00%> (ø)`
pydantic/validators.py	`100.00% <0.00%> (ø)`
pydantic/dataclasses.py	`100.00% <0.00%> (ø)`

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 31bc243...2dbbe5b. Read the comment docs.

PrettyWood

Small typo in changelog. Otherwise LGTM !

changes/1911-TrDex.md

PrettyWood · 2020-09-09T23:11:40Z

@TrDex I reckon you should link this issue #1877 as your PR adds rediss.
You could also go a bit further by changing here ?P<user>[^\s:/]+ into ?P<user>[^\s:/]*.
This way you could add both examples 'rediss://redis' and 'redis://:123@redis' in your tests and fully solve the issue. WDYT ?

Co-authored-by: PrettyWood <em.jolibois@gmail.com>

mykolasolodukha · 2020-09-10T12:17:49Z

@TrDex I reckon you should link this issue #1877 as your PR adds rediss.

Thanks a lot @PrettyWood ! I updated the PR comment (hope it works that way, that's one of my first PRs, lol)

You could also go a bit further by changing here ?P<user>[^\s:/]+ into ?P<user>[^\s:/]*.
This way you could add both examples 'rediss://redis' and 'redis://:123@redis' in your tests and fully solve the issue. WDYT ?

Okay, I'll update that too, and make corresponding tests as well

* Bump black from 19.10b0 to 20.8b1 Bumps [black](https://github.com/psf/black) from 19.10b0 to 20.8b1. - [Release notes](https://github.com/psf/black/releases) - [Changelog](https://github.com/psf/black/blob/master/CHANGES.md) - [Commits](https://github.com/psf/black/commits) Signed-off-by: dependabot[bot] <support@github.com> * fix: run `make format` Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: PrettyWood <em.jolibois@gmail.com>

Refer to #1877

tests/requirements.txt

pydantic/utils.py

samuelcolvin · 2020-10-09T11:51:11Z

pydantic/networks.py

@@ -54,7 +54,7 @@ def url_regex() -> Pattern[str]:
    if _url_regex_cache is None:
        _url_regex_cache = re.compile(
            r'(?:(?P<scheme>[a-z][a-z0-9+\-.]+)://)?'  # scheme https://tools.ietf.org/html/rfc3986#appendix-A
-            r'(?:(?P<user>[^\s:/]+)(?::(?P<password>[^\s/]*))?@)?'  # user info
+            r'(?:(?P<user>[^\s:/]*)(?::(?P<password>[^\s/]*))?@)?'  # user info


this is quite a significant change - allow password without user.

I think it's a good idea but it needs:

to be noted in documentation

to have it's own item in change log

to have tests covering this case.

these items still need resolving.

@TrDex this can't move forward until you act upon this comment.

� Conflicts: � tests/requirements.txt

nuno-andre · 2020-11-05T12:03:39Z

Additionally:

The path, if any, should only be numeric (without leading zeros) and with a single segment.
All fields after the scheme are optional. hostname defaults to localhost, port to 6379 and path to 0.

https://www.iana.org/assignments/uri-schemes/prov/redis

Refer to #1911 (comment) Refer to https://www.iana.org/assignments/uri-schemes/prov/redis

samuelcolvin

Also please fix linting.

samuelcolvin · 2020-11-29T16:01:47Z

pydantic/networks.py

@@ -54,7 +54,7 @@ def url_regex() -> Pattern[str]:
    if _url_regex_cache is None:
        _url_regex_cache = re.compile(
            r'(?:(?P<scheme>[a-z][a-z0-9+\-.]+)://)?'  # scheme https://tools.ietf.org/html/rfc3986#appendix-A
-            r'(?:(?P<user>[^\s:/]+)(?::(?P<password>[^\s/]*))?@)?'  # user info
+            r'(?:(?P<user>[^\s:/]*)(?::(?P<password>[^\s/]*))?@)?'  # user info


@TrDex this can't move forward until you act upon this comment.

pydantic/networks.py

mykolasolodukha · 2020-12-08T20:50:37Z

@samuelcolvin I worked on your comments about allowing password without user.

It's noted in the documentation.
It has its own item in the changelog.
It has tests covering this case.

tests/test_networks.py

* Add `rediss` (Redis over SSL) protocol to `RedisDsn` * Update docs * Update changes history * Fix typo in `changes/1911-TrDex.md` Co-authored-by: PrettyWood <em.jolibois@gmail.com> * Bump black from 19.10b0 to 20.8b1 (#1909) * Bump black from 19.10b0 to 20.8b1 Bumps [black](https://github.com/psf/black) from 19.10b0 to 20.8b1. - [Release notes](https://github.com/psf/black/releases) - [Changelog](https://github.com/psf/black/blob/master/CHANGES.md) - [Commits](https://github.com/psf/black/commits) Signed-off-by: dependabot[bot] <support@github.com> * fix: run `make format` Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: PrettyWood <em.jolibois@gmail.com> * Allow URLs without `user` part Refer to #1877 * Update docs * Allow Redis DSN with schema only Refer to pydantic/pydantic#1911 (comment) Refer to https://www.iana.org/assignments/uri-schemes/prov/redis * Fix lint error * Fix lint error * Set `parts: Dict[str, Optional[str]]` instead of `Dict[str, str]` * Fix linting * More verbose default values set in `RedisDsn.validate_parts()` * Fix linting 2 * Fix typo in docs * Add a note in the changelog * Add test case for URL without `user` part * change port in test Co-authored-by: PrettyWood <em.jolibois@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Samuel Colvin <s@muelcolvin.com>

mykolasolodukha added 2 commits September 7, 2020 20:49

Add rediss (Redis over SSL) protocol to RedisDsn

972cf8c

Update docs

6cc9d92

Update changes history

5605294

mykolasolodukha marked this pull request as ready for review September 7, 2020 18:27

PrettyWood reviewed Sep 9, 2020

View reviewed changes

changes/1911-TrDex.md Outdated Show resolved Hide resolved

Fix typo in changes/1911-TrDex.md

eec3cef

Co-authored-by: PrettyWood <em.jolibois@gmail.com>

dependabot bot and others added 2 commits September 10, 2020 15:20

Allow URLs without user part

035b181

Refer to #1877

samuelcolvin reviewed Oct 9, 2020

View reviewed changes

Merge branch 'master' of github.com:/samuelcolvin/pydantic into master

cc688f4

� Conflicts: � tests/requirements.txt

PrettyWood mentioned this pull request Nov 5, 2020

Add "rediss" scheme. #2093

Closed

mykolasolodukha added 7 commits November 10, 2020 10:01

Merge remote-tracking branch 'upstream/master'

cc9c018

Update docs

e1ab6da

Allow Redis DSN with schema only

953d577

Refer to #1911 (comment) Refer to https://www.iana.org/assignments/uri-schemes/prov/redis

Fix lint error

fd017dd

Fix lint error

0992901

Merge remote-tracking branch 'origin/master'

b720f6f

Set parts: Dict[str, Optional[str]] instead of Dict[str, str]

b5acc31

samuelcolvin reviewed Nov 29, 2020

View reviewed changes

mykolasolodukha added 6 commits December 8, 2020 22:13

Fix linting

c85351b

More verbose default values set in RedisDsn.validate_parts()

16c9699

Fix linting 2

b23b1ed

Fix typo in docs

4b2452f

Add a note in the changelog

7543a75

Add test case for URL without user part

6daeefc

PrettyWood reviewed Dec 18, 2020

View reviewed changes

tests/test_networks.py Outdated Show resolved Hide resolved

change port in test

2dbbe5b

samuelcolvin merged commit aacf592 into pydantic:master Dec 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add `rediss` (Redis over SSL) protocol to `RedisDsn` #1911

Add `rediss` (Redis over SSL) protocol to `RedisDsn` #1911

Uh oh!

mykolasolodukha commented Sep 7, 2020 •

edited

Loading

Uh oh!

codecov bot commented Sep 7, 2020 •

edited

Loading

Uh oh!

PrettyWood left a comment

Uh oh!

Uh oh!

PrettyWood commented Sep 9, 2020 •

edited

Loading

Uh oh!

mykolasolodukha commented Sep 10, 2020

Uh oh!

Uh oh!

Uh oh!

samuelcolvin Oct 9, 2020 •

edited

Loading

Uh oh!

samuelcolvin Oct 25, 2020

Uh oh!

samuelcolvin Nov 29, 2020

Uh oh!

nuno-andre commented Nov 5, 2020

Uh oh!

samuelcolvin left a comment

Uh oh!

samuelcolvin Nov 29, 2020

Uh oh!

Uh oh!

mykolasolodukha commented Dec 8, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Add rediss (Redis over SSL) protocol to RedisDsn #1911

Add rediss (Redis over SSL) protocol to RedisDsn #1911

Uh oh!

Conversation

mykolasolodukha commented Sep 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change Summary

Related issue number

Checklist

Uh oh!

codecov bot commented Sep 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

PrettyWood left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PrettyWood commented Sep 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mykolasolodukha commented Sep 10, 2020

Uh oh!

Uh oh!

Uh oh!

samuelcolvin Oct 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samuelcolvin Oct 25, 2020

Choose a reason for hiding this comment

Uh oh!

samuelcolvin Nov 29, 2020

Choose a reason for hiding this comment

Uh oh!

nuno-andre commented Nov 5, 2020

Uh oh!

samuelcolvin left a comment

Choose a reason for hiding this comment

Uh oh!

samuelcolvin Nov 29, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mykolasolodukha commented Dec 8, 2020

Uh oh!

Uh oh!

Uh oh!

Add `rediss` (Redis over SSL) protocol to `RedisDsn` #1911

Add `rediss` (Redis over SSL) protocol to `RedisDsn` #1911

mykolasolodukha commented Sep 7, 2020 •

edited

Loading

codecov bot commented Sep 7, 2020 •

edited

Loading

PrettyWood commented Sep 9, 2020 •

edited

Loading

samuelcolvin Oct 9, 2020 •

edited

Loading