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 <[email protected]> Gerrit-Reviewer: Fabian Deutsch <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ryan Barry <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: No _______________________________________________ node-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/node-patches
