AfterShip / email-verifier

:white_check_mark: A Go library for email verification without sending any emails.
MIT License
1.18k stars 149 forks source link

Adding DisableCatchAllCheck() switch #76

Closed lguminski closed 1 year ago

lguminski commented 1 year ago

closes #75

lryong commented 1 year ago

Hi @lguminski Since the catchAll check actually requires the smtpCheck switch to be turned on, your PR implementation is more like a sub-switch. Can you do some optimization? For example, by placing a cache for the check results, you can avoid duplicate catchAll checks for the domain multiple times

lguminski commented 1 year ago

Hi @lryong,

Since the catchAll check actually requires the smtpCheck switch to be turned on, your PR implementation is more like a sub-switch.

Indeed it is a subswitch.

Can you do some optimization? For example, by placing a cache for the check results, you can avoid duplicate catchAll checks for the domain multiple times

Sadly caching is not enough for my use case. Let me explain. I am performing batch processing of a large number of emails (say 512 emails). For performance reasons, the input list of email addresses is split into chunks of 32 emails, and each chunk gets its own worker (512/32 = 16 workers) to validate them. Workers are separate processes that don't share memory.

So caching would be an optimisation, but still, each of the 16 workers would perform one catchAll check. Whereas I want to disable it completely.

git-hulk commented 1 year ago

Hi @lryong, I think it makes a lot of sense to allow disabling the catch-all feature if it's NOT needed.

And for this PR, @lguminski it'll be good if you can help to pass the CI lint.

lryong commented 1 year ago

OK, I would make the CI lint pass

lryong commented 1 year ago

@lguminski Please pull down and merge the latest code, I've updated the test cases to make sure the CI passes

lguminski commented 1 year ago

@lryong thanks for fixing tests. Everything looks now good to me

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 4312954800


Changes Missing Coverage Covered Lines Changed/Added Lines %
verifier.go 7 11 63.64%
smtp.go 12 17 70.59%
<!-- Total: 19 28 67.86% -->
Totals Coverage Status
Change from base Build 4300857021: -0.3%
Covered Lines: 527
Relevant Lines: 622

💛 - Coveralls