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

Improved py36 support #202

Merged
merged 1 commit into from Jun 26, 2018
Merged

Improved py36 support #202

merged 1 commit into from Jun 26, 2018

Conversation

eserge
Copy link
Contributor

@eserge eserge commented Jun 20, 2018

The changes made here are dealing with the differences between iteration functions in py3 and py2. Specifically, dnsq returns a result of a filter function from its mx_hosts_for function. In Python 2 it's a list, but for py3 it's an iterator. Later flanker tries to get a len of that list which is impossible with an iterator. So this pull request deals with this by converting iterator to a list.

Other than that there are some improvements in tests.

@mailgun-ci
Copy link

Can one of the admins verify this patch?

@eserge
Copy link
Contributor Author

eserge commented Jun 20, 2018

Yes, sure. If this goes well, I'm planning to make couple more commits

@@ -11,6 +12,9 @@
from flanker.addresslib import address, validate


PY3 = sys.version_info[0] == 3
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everywhere else we use six.PY3 instead, please do the same for consistency.

@horkhe horkhe requested review from horkhe and b0d0nne11 June 20, 2018 19:03
@eserge
Copy link
Contributor Author

eserge commented Jun 25, 2018

Made the change about six.PY3 that you requested. Also reverted expanded if...else back to ternary. It seems to me that the part of the condition where there is a dot at the end is not reachable by the execution, because the domain comes to this function after being processed by the email validation and it doesn't seem to allow domains in FQDN form with dot at the end.

Also reverted version change in setup.py as to not to impose any particular version for this change to be released in.

@horkhe
Copy link
Member

horkhe commented Jun 26, 2018

Could you please squash your commits, and then I will merge.

@eserge
Copy link
Contributor Author

eserge commented Jun 26, 2018

Done

@horkhe horkhe merged commit 57701a6 into mailgun:master Jun 26, 2018
@horkhe
Copy link
Member

horkhe commented Jun 26, 2018

@eserge thank you for your contribution.

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 this pull request may close these issues.

None yet

3 participants