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

Idea: cache successful dns lookups? #55

Closed
HeyHugo opened this issue Oct 16, 2020 · 4 comments · Fixed by #58
Closed

Idea: cache successful dns lookups? #55

HeyHugo opened this issue Oct 16, 2020 · 4 comments · Fixed by #58

Comments

@HeyHugo
Copy link
Contributor

HeyHugo commented Oct 16, 2020

Would it make sense to cache (in memory) dns records that got a match to prevent a large amount of dns lookups for common email providers like gmail.com, hotmail.com etc?

If also caching non matches is fine functools.lru_cache decorator could be added to validate_email_deliverability, otherwise some custom logic could be written to keep a small cache of recent valid domains.

What do you think? I could probably make a PR if you like it?

@JoshData
Copy link
Owner

It makes sense. But the caller should have control over the cache lifetime -- it shouldn't be global or held in memory forever.

@HeyHugo
Copy link
Contributor Author

HeyHugo commented Oct 16, 2020

I think a LRU type of cache would be quite fitting. It'd mean frequent occurring domains would stay in cache most of the time.
I'm thinking something quite simple like this could work: https://gist.github.com/damzam/4b0812c997e91f1bed17

With that said if you like the idea of having it timeout based instead I'm not opposed. That could work by storing each domain together with a timestamp. Cleanup of expired records could be done when encountering an expired domain.

What do you say, LRU cache or timeouts, should I make a PR?

@JoshData
Copy link
Owner

The best way would probably be to have the user pass in a dnsresolver cache object (https://dnspython.readthedocs.io/en/stable/resolver-caching.html) and then pass that to the resolver.

@HeyHugo
Copy link
Contributor Author

HeyHugo commented Oct 17, 2020

Ah neat so dnspython has built in caching support. Makes sense to use that then 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants