Skip to content
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

TLD list testing added #282

Closed
wants to merge 7 commits into from
Closed

Conversation

aviiciii
Copy link
Contributor

@aviiciii aviiciii commented Jul 30, 2023

The TLD in the domain validator are checked against a valid list of TLDs from "https://data.iana.org/TLD/tlds-alpha-by-domain.txt"

This pull request solves #198

validators/domain.py Fixed Show fixed Hide fixed
@aviiciii
Copy link
Contributor Author

The tests on urls.py seems to fail. I'm not sure if changes on domain.py affect the latter.

Also, the domain validator depends on the internet to fetch the list of TLDs, Do we need to have a static list in case the data can't be pulled?

@yozachar
Copy link
Collaborator

yozachar commented Jul 30, 2023

The tests on urls.py seems to fail. I'm not sure if changes on domain.py affect the latter.

Yes, URLs are composed of either ip-address or domain.

Also, the domain validator depends on the internet to fetch the list of TLDs, Do we need to have a static list in case the data can't be pulled?

I'd prefer caching TLDs.


For now, I'll put this PR on hold, because:

  1. It introduces an external dependency (which in this case, it can be resolved fairly easily)
  2. Needs a bit of complex approach (like caching)
  3. The PR seeks to address issues mentioned in Why msn.comm validates? #67 & invalid email addresses but validators return True #28 (comment) which is more like answering the question "Does this flower exists today?" than answering "Is that a flower?". The former is a bit more nuanced.
  4. I'm working on a couple of core changes that would effect every upcoming PR.

Feel free to work on 1 & 2 if you like, keeping in mind that 4 could require a drastic change in this PR.

@yozachar yozachar marked this pull request as draft July 31, 2023 04:20
@aviiciii
Copy link
Contributor Author

Ok cool, let me look what I can do on 1 and 2

@yozachar
Copy link
Collaborator

yozachar commented Aug 1, 2023

Ok cool, let me look what I can do on 1 and 2

  1. You can use Python's standard library to send request.
  2. Look into caching in Python.

@yozachar yozachar force-pushed the master branch 7 times, most recently from c5f8f78 to 73d540f Compare August 7, 2023 06:05
@yozachar yozachar added the enhancement Issue/PR: A new feature label Aug 7, 2023
@davidt99
Copy link
Contributor

Just want to bring the attention to this library that already handle this, maybe we can use it or take inspiration:
https://github.com/john-kurkowski/tldextract

@yozachar yozachar added the outdated Issue/PR: Open for more than 3 months label Nov 7, 2023
@yozachar yozachar closed this in #360 Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issue/PR: A new feature outdated Issue/PR: Open for more than 3 months
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants