[node-patches] Change in ovirt-node[master]: Use a better URL validator. RegexValidator adds flags

[rbarry at redhat.com]

Ryan Barry has posted comments on this change.

Change subject: Use a better URL validator. RegexValidator adds flags
......................................................................


Patch Set 2:

Changing the __init__ of the validator isn't strictly necessary (we could use the same strategy as FQDN, which also used a flag). It simply seemed more obvious to do it explicitly than to expand a tuple, and provides a clear pattern for adding flags to other validators (if we need them)

There's not a practical solution for EL6. urlparse sees "http://foo::" as part of the netloc, which is incorrect. We could explicitly cover the double colon case, but that leads us down a potential rabbit hole of someone finding another case where the URL validator breaks and us chasing it down. I can already tell you that urlparse doesn't have a problem with "--" in a domain, though it's invalid, and we'd need to catch that case as well.

urllib3 is a viable option once EL6 support is over. And I agree that this would be difficult to modify. But validating URLs correctly is very hard, and passing the existing doctests is a good indicator that it won't need to be modified anywhere. Additionally, since the regex came from the django project (I didn't write it), updating is easy.

I added a very complete doctest suite.

-- 
To view, visit https://gerrit.ovirt.org/42394
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I13429d0cc600446c8fd4c1be5e5a8752d6d992f7
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Ryan Barry <rbarry at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ryan Barry <rbarry at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: No