-
-
Notifications
You must be signed in to change notification settings - Fork 66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
'example.de' (and many others hostnames) not valid with hostname format #26
Comments
This is the current regex:
As far as I see it, this means, the last part can never be exactly 2 chars long. This is a huge problem, as there are many tlds with 2 chars. (.de, .fr, etc.) In the PR I'll probably use a different regex. But: As I said in the other issue today: I don't think regexes are a good solution for formats that can easily be validated using python standard library means (other than |
I agree, some formats would be better to validate by some extra library than by regexps. Also, some regexps are are not super correct for some cases. But it brings dependencies which I want to avoid until it's not really necessary. :-) But maybe, if license allows it, I could copy-paste some code into this library to keep an eye on performance and avoid problems with changing dependencies. Will put on my todo list to check possibilities. |
In some cases, like ipv4 and ipv6, you could use functionality like the ipaddress module that doesn't require additional dependencies. |
Switched to different hostname regex. Fixes #26
@horejsek Thanks for merging! I would be very happy if you can do a bugfix release with this in the near future, because this issue keeps us from rolling out a component that we switched from jsonschema to your library (because of draft7 support, performance and I like it better :) ) |
Done, v2.3 :-) |
Glad it helped your component! :-) |
Aaand another one. Hope you aren't getting annoyed. Running this example throws a JsonSchemaException:
For 'google.com' it works fine. I'll create a PR soon, probably.
The text was updated successfully, but these errors were encountered: