django 2.2-3.0 -- py3 (3.5-3.8) only

max-arnold / python-block-disposable-email

Python client for http://block-disposable-email.com

Other

0 stars 1 forks source link

django 2.2-3.0 -- py3 (3.5-3.8) only #2

Closed jheld closed 4 years ago

jheld commented 4 years ago

Although it seems this repo works with newer versions of django and python, it would be great to make it explicit.

Is this something you might have time to work on? If not, I can try to put in some time on this but wanted to check first.

max-arnold commented 4 years ago

I no longer use this library, but I'll be happy to accept a PR. Am I correct that you want to improve the test suite so it runs on different python/django versions?

EDIT: I also think it is fine to make it Py3 only.

jheld commented 4 years ago

Hi @max-arnold I appreciate your feedback!

Test suite (probably) would be improved -- my initial goal yes is py3 only (I recently took ownership of the django block disposable email repo so I was thinking more in terms of django, but yes, py3!), and more specifically (if possible), 3.5-3.8. So changing the code, seeing if the tests keep passing, and seeing how "little" work I can do to get it passing if it doesn't initially.

I'll give it a shot probably within the next couple of weeks. The django repo (which uses this repo) did pass its tests, and my company has been using that repo (and thus this one) even with py3.6 and 3.7, so unless we have been fortunate not to hit certain bad/incompatible code paths, it's pretty likely that this will be a smooth upgrade all around.

jheld commented 4 years ago

@max-arnold please see #3 , it is failing on py38...maybe because of gnureadline ? unsure what's going on there -- that project in the description says it has support for 3.8.

jheld commented 4 years ago

The failure occurs on my branch on the PR on my fork (adds github action tests): https://github.com/jheld/python-block-disposable-email/pull/1

max-arnold commented 4 years ago

@raicheff Pinging you too, because your flask-bdea library uses unpinned block-disposable-email dependency and we are going to drop Py2 support.

max-arnold commented 4 years ago

@jheld I merged your PR and also pushed a couple of fixes on top:

Split requirements to multiple files (via pip-tools) - those are mostly for development purposes, so the gnureadline library is not really necessary for the test environment
Ability to pass a real API key to the test suite
Ensure that tox runs locally with all the specified Django versions + a development one

Also I'd like to get your input on these two questions:

What is your suggestion for a version bump? 1.0.2 or something like 1.1.0 (or even 2.0) to indicate that we are dropping Py2 support?
I noticed the BDEA_MESSAGE and BDEA_TIMEOUT settings, and also a whitelist/fallback logic here: https://github.com/jheld/DisposableEmailChecker/blob/master/disposable_email_checker/validators.py If you are interested in upstreaming those, please open another issue so we can discuss it

jheld commented 4 years ago

glad to hear this! yeah I'd prefer 2.0 (if you want to issue an alpha or beta version for people to test that is fine, too).

i will try to review your upstream suggestion within the next day.

jheld commented 4 years ago

Currently I'm having a hard time understanding the differences in the validators (why my project took one on). Maybe some error handling/easier logic setup? And yes especially because this project also requires/supports django, I agree it seems pretty reasonable to consolidate the tooling/utilities, and in particular the validation pieces.

I'll setup the issue.

max-arnold commented 4 years ago

Closing this issue. I'll bump the version number once we decide on #4