Django 1.7 and Python3 compatibility

[Luthaf]

This fix compatibility mode for Django1.7 and Python 3.

[Luthaf]

Tests mainly pass with Python 3.4 and 2.7 (Django 1.7 every time). The only failure is in this function. The traceback is

FAIL: test__unicode__ (background_task.tests.TestBackgroundDecorator)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Volumes/Aldith/Developpeur/django-background-task/background_task/tests.py", line 72, in test__unicode__
    unicode(proxy))
AssertionError: 'TaskProxy(background_task.tests.empty_task)' != '<background_task.tasks.TaskProxy object at 0x1086423c8>'
- TaskProxy(background_task.tests.empty_task)
+ <background_task.tasks.TaskProxy object at 0x1086423c8>

Is this test really important ?

[Luthaf]

Bump. If you don't want to have this, I can use my own fork.

[SonOfLilit]

@Luthaf thanks!

@lilspikey are there plans to merge this?

[Luthaf]

Bump. Can you please merge it, or state clearly that you do not whant it ? I just ran into this bug while trying to deploy a server.

[lilspikey]

Sorry, been bad timing with this. I became a Dad again on the 1st of
September and work has been hectic too. As I've not personally been using
Django 1.7 or Python 3 this hasn't been a priority for me...

I would like to see this merged, but I've not got a lot of time right now.

RE: the failing test. I think this is down to difference between str
and unicode in Python 3?

Re: removing autocommit - which version of Django deprecated that? I've
got large sites that are on older versions of Django using this, so don't
want to break things if I can avoid it. Might need to add in some sort of
compatibility layer for that.

Hopefully we can work together to get this sorted.

cheers,

John

On 10 March 2015 at 21:35, Luthaf notifications@github.com wrote:

Bump. Can you please merge it, or state clearly that you do not whant it ?
I just ran into this bug while tying to deploy a server.

—
Reply to this email directly or view it on GitHub
#12 (comment)
.

http://psychicorigami.com/

[Luthaf]

Congratulations !

I'll have a look about the failing test.

autocommit was deprecated in django 1.6 I think, and is now the default. See https://docs.djangoproject.com/en/1.7/topics/db/transactions/. I can add a small wrapper around this to check for Django version.

[Luthaf]

Ok, test is fixed, and wrapper for Django compatibility is made !

Your turn now =)

[lilspikey]

Ok, just started looking at this.

Failing on the first hurdle with tests though.

I'm running this against Django 1.4.3 (just happens to be a version I have in a virtual env).

• Tests script changes assume that both python2 and python3 installed - I used virtualenv normally, so effectively only one version of python at a time. I'd suggest just use "python" instead of "python2" and "python3" and then use virtualenv to be able to test both.
• The autocommit compatibility check fails the moment you have a version number that cannot be parsed as a float (e.g. 1.4.3). Also don't forget that version numbers may contain letters... You should instead check to see if the autocommit function exists or not (see code below):

if hasattr(transaction, 'autocommit'):
    compatibility_autocommit = transaction.autocommit
else:
    # Autocommit is deprecated and the default
    def compatibility_autocommit(function):
        def wrapper(*args, **kwargs):
            return function(*args, **kwargs)
        return wrapper

• You've changed get_query_set to get_queryset, which doesn't exist prior to Django 1.6 - so I've got failing tests. We need a compatibility layer for that - maybe just use all() instead of get_queryset?

Might be some other issues, but that's what I've got so far. Figured I had a spare half out, so I'd have a quick look.

cheers,

John

[Luthaf]

I'm running this against Django 1.4.3

Are you sure you still need compatibility for this version of Django ? The 1.6 version has been deprecated by the Django team recently.

You should instead check to see if the autocommit function exists or not

It will exist in all the cases, as it is only deprecated for now.

All the other comments are now fixed !

[lilspikey]

Django 1.4 is a long term release (support for three years), so it's still
supported. So I would like to support it in this - particularly given I
have production code running with Django 1.4.

On 30 April 2015 at 22:40, Luthaf notifications@github.com wrote:

I'm running this against Django 1.4.3

Are you sure you still need compatibility for this version of Django ? The
1.6 version has been deprecated by the Django team recently.

You should instead check to see if the autocommit function exists or not

It will exist in all the cases, as it is only deprecated for now.

All the other comments are now fixed !

—
Reply to this email directly or view it on GitHub
#12 (comment)
.

http://psychicorigami.com/

[Luthaf]

Ok, I didn't knew that. Tell me if you have more comments on this PR !

[lilspikey]

Ok, so that seems to all pass tests ok for Django 1.4 now.

I'd suggest changing the unlock method (to make it a bit more legible from):

    def unlocked(self, now):
        max_run_time = getattr(settings, 'MAX_RUN_TIME', 3600)
        if LooseVersion(django.get_version()) > LooseVersion("1.6"):
            qs = self.get_queryset()
        else:
            qs = self.get_query_set()
        expires_at = now - timedelta(seconds=max_run_time)
        unlocked = Q(locked_by=None) | Q(locked_at__lt=expires_at)
        return qs.filter(unlocked)

To this:

    def unlocked(self, now):
        max_run_time = getattr(settings, 'MAX_RUN_TIME', 3600)
        qs = self.all()
        expires_at = now - timedelta(seconds=max_run_time)
        unlocked = Q(locked_by=None) | Q(locked_at__lt=expires_at)
        return qs.filter(unlocked)

Basically replace the call to get_queryset() with all(), so you can avoid having a version check in the middle there (which makes things look messy).

[simonv3]

Hey @lilspikey & @Luthaf, is that last change all that's needed to get this merged in? If neither of you have time I can quickly make that change.

[Luthaf]

If you want to do it, I'll merge any PR to my fork (which will update this one). I effectively don't have time for this.

[simonv3]

I've since found this fork that works on Django 1.7 & Python 3: https://github.com/arteria/django-background-tasks

[Luthaf]

Closing this as their is no chance to get it merged, and I am not using this code anymore. For anyone interested in it, please see the fork linked by @simonv3.