Python 3 and django 1.5 compatability #71

pydanny · 2013-07-26T13:46:35Z

This pull request does the following:

Adds in Python 3.3+ compatibility with Python 2.7.x
Adds in Django 1.5 custom user model compatibility while maintaining compatibility with Django 1.4 or less.
Fixes some issues with DateField vs DateTimeField which broke hard on Python 3.

What this pull request doesn't do:

There may be several DateField vs DateTimeField issues remaining. I'll investigate and possibly mitigate those in a future pull request.

…jango 1.5+ custom user models

paltman · 2013-07-26T17:48:58Z

payments/models.py

+ pass
+ finally:
+ pass
+ return self.current_subscription.is_valid()


Why did you move this from outside of the exception block where if the CurrentSubscription didn't exist it would return False instead of blow up with an Exception?

I think I confused myself when I was looking at it. And it passed the tests in the new configuration. I'll return it to it's previous state and run the tests.

Could use a test for that case. Coverage isn't there yet. Been adding tests as I add or change things or regressions are reported.

Sent from my iPhone

On Jul 26, 2013, at 1:39 PM, Daniel Greenfeld [email protected] wrote:

In payments/models.py:

except CurrentSubscription.DoesNotExist: return False

except Exception as e:

raise

else:

pass

finally:

pass

return self.current_subscription.is_valid()
I think I confused myself when I was looking at it. And it passed the tests in the new configuration. I'll return it to it's previous state and run the tests.

—
Reply to this email directly or view it on GitHub.

paltman · 2013-07-26T18:00:38Z

@pydanny thanks for the PR, still reviewing some bits, but could you go ahead an address the comments I already left as well as investigate the bulid failure? Would be great if you could add Python 3 directives in the .travis.yml file while you are at it.

Thanks!

pydanny · 2013-07-26T19:00:19Z

In addition to the inline changes, I also added Python 3.3 to the .travis.yml file. I could also do 3.2 but in my experiences getting production level Django to work well with 3.2 can be finicky. We ignore that iteration of Python. Up to you...

pydanny · 2013-07-26T19:01:49Z

Oops - Travis is running 3.3 against Django 1.4, which will break. Let me see what I can do with the config file

…s, and removed unnecessary QueryDict import

paltman · 2013-07-26T21:53:36Z

payments/managers.py



 class CustomerManager(models.Manager):

 def started_during(self, year, month):
+ # Need to implement datetime range because 'start' field is DateTimeField
+ start_date = timezone.datetime(year, month, 1, tzinfo=timezone.utc)
+ end_date = timezone.datetime(year, month + 1, 1, tzinfo=timezone.utc)


@pydanny what happens in December? :)

Can you add some tests for this manager method (and the other datetime methods you are changing)?

pydanny · 2013-07-28T19:04:11Z

Should be good now.

pydanny · 2013-07-28T20:09:30Z

it's failing on the pylint and there is nothing I can do to get around it. Problems:

I've added whitespace to the test_managers.py module and something constantly removes it.
pylint doesn't like working with Django 1.5 user models

I'm stuck and can't get past it.

pydanny · 2013-07-30T21:57:28Z

@paltman Any movement on this? I'm getting stuck on the Eldarion linter which doesn't seem to like Django 1.5 custom user model fix and so throws an error.

paltman · 2013-07-30T22:13:31Z

payments/settings.py

 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
 from django.utils import importlib

+try:
+ from django.contrib.auth import get_user_model


@pydanny you can add a disable message:

from django.contrib.auth import get_user_model # pylint: disable-msg=E0611

pydanny · 2013-07-30T22:16:35Z

I'm on it!

paltman · 2013-07-30T22:17:37Z

@pydanny the error in the 3.3 travis run looks like an issue with the version of distribute under python 3.3 when running the setup.py. I don't have a Python 3 environment locally to try an reproduce but you might try installing locally in a new virtualenv the setup.py and debug from there.

pydanny · 2013-07-30T22:20:34Z

Okay, just set up new environment and I'm getting the same problem. But not in my old one. How weird? Okay... digging in.

paltman · 2013-07-30T22:23:51Z

@pydanny from the stack trace it looks like something in building the long description, so I wonder if it's something in this function that Python 3 doesn't like:

https://github.com/eldarion/django-stripe-payments/blob/master/setup.py#L10

paltman · 2013-07-30T22:24:41Z

The three other lint errors seem to be indented white space issues in test_managers.py that should be trivial to fix.

pydanny · 2013-07-30T23:02:16Z

All tests pass locally on all tested versions of Python.

On travis, everything but Python 3 passes, and Python 3 fails because Pylint does't work with Python 3. See https://travis-ci.org/eldarion/django-stripe-payments

paltman · 2013-07-31T02:08:46Z

Hm... I wonder if this is helpful http:https://stackoverflow.com/questions/10772824/no-luck-pip-installing-pylint-for-python-3

I don't have much time to debug this right now (this week is crazy busy for me). I realize this could likely just be a Travis issue but really don't want to merge under we sort out Travis / pylint issues. I dig into this as soon as I have time to figure out why pylint is breaking in this configuration as docs seem to indicate pylint is supported on Python 3. Might not be until next week though.

brosner · 2013-07-31T04:30:26Z

payments/managers.py

 return self.exclude(
 current_subscription__status="trialing"
 ).filter(
- current_subscription__start__year=year,
- current_subscription__start__month=month
+ current_subscription__start__range=get_range(year, month),


Can you explain to me in more detail why this change is required? I fired up a 1.5 project with Python 2 and 3 to see if I could see any reason for this change, but I am not finding any. The Django documentation doesn't even mention the need for this kind of change.

Because django-stripe-payments mixes Date calculations with DateTime fields. It uses DateTimeField but treats them like DateFields. I was actually surprised this wasn't opened as an issue earlier.

There is actually a lot of stuff about this available, including in the Django docs, but for now there is http:https://stackoverflow.com/a/10048320/93270

I don't see what I was doing as mixing "Date calculation with DateTime fields". I am merely filtering a DateTimeField by it's year and month component. I don't see what the the problem is with that. Granted I haven't used Python 3 at all, but also not sure why Python 3 would/should change how a queryset of a DateTimeField works.

The issue reported in the StackOverflow link is not really relevant. I am not saying there isn't an issue here. I really want be shown what it is though. The problem on SO is that poster is trying to compare datetimes in the databases to a very specific datetime or a date object. It is easy to see that won't work because equality checks will require an exact match. When Django is given a date object to do a datetime comparison it will just fill in the time with midnight.

We are not doing that in the code you've changed. We are using __year and __month lookups which result in SQL like:

WHERE (EXTRACT(\'month\' FROM "product_subscription"."start") = 6 AND "product_subscription"."start" BETWEEN 2013-01-01 00:00:00 and 2013-12-31 23:59:59.999999)

Which doesn't suffer from the problem being reported on SO.

I assume you made this change because you saw a real issue. If you can help me reproduce this issue I'd be glad to accept a fix for it.

Furthermore, I don't see how this is relevant to goal of the PR which is add Python 3 and Django 1.5 support.

I thought I had a failure in the tests on Python 3, hence the change. I'll undo the changes and see what happens.

pydanny · 2013-07-31T14:37:22Z

Since Eldarion insists on using a linter in Travis-CI, I'll let Eldarion address the issue.

Hate to go this way, but if my deadline gets much closer, I'll need to port this library and release it as a stable dependency on PyPI.

paltman · 2013-07-31T15:00:29Z

That's fine. You have to do what you have to do when you have deadlines. I don't really feel comfortable removing the linter just to support this case.

pydanny · 2013-08-01T09:56:14Z

I got the deadline pushed back a couple weeks for so I should have time to see this to the end.

pydanny · 2013-08-02T13:16:36Z

Okay, back to it:

@brosner pointed out that remove u"" meant that Python 2.7 wouldn't work. So I 'fixed' that, forgetting __future__ import unicode_literals. I'll recorrect that shortly
I'll check the DateField/DateTimeField problem I thought I discovered and report my findings.

pydanny · 2013-08-04T10:18:54Z

Client has agreed to move to Python 2.7 for now as there are too many packages to update to get project working on Python 3. Which means I'm closing this pull request for now. I might try and work it again in the future.

Fix sync account next

pydanny added 3 commits July 26, 2013 14:22

First pass on making django-stripe-payments work for Python 3/2 and D…

4c99ba3

…jango 1.5+ custom user models

Fixed the bytes-to-str in the webhook

c870d2b

Fix date vs datetime range problems

100eaa9

paltman reviewed Jul 26, 2013
View reviewed changes

pydanny added 5 commits July 26, 2013 21:02

Fixed has_active_subscription try/except, adding settings.User import…

cc3fe3f

…s, and removed unnecessary QueryDict import

excluding Django 1.4 and Python 3.3 from tests

49f0e39

excluding Django 1.4 and Python 3.3 from tests

b58baf6

excluding Django 1.4 and Python 3.3 from tests

f6a210f

Another attempt to get travis to play nice

e655361

paltman reviewed Jul 26, 2013
View reviewed changes

pydanny added 2 commits July 27, 2013 00:54

get_year_month_range function plus tests

5861038

added some tests for the range builder and the during manager method

144dc95

pydanny added 5 commits July 28, 2013 21:04

Display of __unicode__ methods as __str__ for Python 3 users

0197eb8

changes to match pinax standard of PEP8

d7ab31c

Another PEP8 correction set

ba1f12e

White space PEP8 stuff

f835292

Another attempt to fix things to match pylint

f51d4d5

paltman reviewed Jul 30, 2013
View reviewed changes

pydanny added 4 commits July 31, 2013 00:39

small compat fixes

69dd8bd

small compat fixes

6db9375

Make Python 3 work for develop

6c37597

Another fine mess

a484642

brosner reviewed Jul 31, 2013
View reviewed changes

Returning the __unicode__ methods to proper handling

631cc66

pydanny closed this Aug 4, 2013

ticosax pushed a commit to ticosax/pinax-stripe that referenced this pull request Nov 15, 2017

Merge pull request pinax#71 from blueyed/fix-sync-account-next

ffd20dc

Fix sync account next

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python 3 and django 1.5 compatability #71

Python 3 and django 1.5 compatability #71

pydanny commented Jul 26, 2013

paltman Jul 26, 2013

pydanny Jul 26, 2013

paltman Jul 26, 2013

paltman commented Jul 26, 2013

pydanny commented Jul 26, 2013

pydanny commented Jul 26, 2013

paltman Jul 26, 2013

paltman Jul 26, 2013

pydanny commented Jul 28, 2013

pydanny commented Jul 28, 2013

pydanny commented Jul 30, 2013

paltman Jul 30, 2013

pydanny commented Jul 30, 2013

paltman commented Jul 30, 2013

pydanny commented Jul 30, 2013

paltman commented Jul 30, 2013

paltman commented Jul 30, 2013

pydanny commented Jul 30, 2013

paltman commented Jul 31, 2013

brosner Jul 31, 2013

pydanny Jul 31, 2013

paltman Jul 31, 2013

brosner Jul 31, 2013

pydanny Aug 1, 2013

pydanny commented Jul 31, 2013

paltman commented Jul 31, 2013

pydanny commented Aug 1, 2013

pydanny commented Aug 2, 2013

pydanny commented Aug 4, 2013

Python 3 and django 1.5 compatability #71

Python 3 and django 1.5 compatability #71

Conversation

pydanny commented Jul 26, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paltman commented Jul 26, 2013

pydanny commented Jul 26, 2013

pydanny commented Jul 26, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pydanny commented Jul 28, 2013

pydanny commented Jul 28, 2013

pydanny commented Jul 30, 2013

Choose a reason for hiding this comment

pydanny commented Jul 30, 2013

paltman commented Jul 30, 2013

pydanny commented Jul 30, 2013

paltman commented Jul 30, 2013

paltman commented Jul 30, 2013

pydanny commented Jul 30, 2013

paltman commented Jul 31, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pydanny commented Jul 31, 2013

paltman commented Jul 31, 2013

pydanny commented Aug 1, 2013

pydanny commented Aug 2, 2013

pydanny commented Aug 4, 2013