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

Django 2 compatiblity #279

Merged
merged 9 commits into from
Jan 8, 2018
Merged

Django 2 compatiblity #279

merged 9 commits into from
Jan 8, 2018

Conversation

Eagllus
Copy link
Collaborator

@Eagllus Eagllus commented Dec 12, 2017

This is not a new branch I just merged master to see if that fixes most of the Django 2.0 problems.

And I was unable to run the tests local so decided to create a PR. that just checks if master + django_2_compat is 👍!

Edit 1: Well master + django_2_compat din't solve all problems.
Edit 2: Fixed a few of the bugs so far.

@codecov-io
Copy link

codecov-io commented Dec 12, 2017

Codecov Report

❗ No coverage uploaded for pull request base (django_2_compat@72b52b0). Click here to learn what that means.
The diff coverage is 89.53%.

Impacted file tree graph

@@                Coverage Diff                 @@
##             django_2_compat     #279   +/-   ##
==================================================
  Coverage                   ?   90.25%           
==================================================
  Files                      ?       44           
  Lines                      ?     2832           
  Branches                   ?        0           
==================================================
  Hits                       ?     2556           
  Misses                     ?      276           
  Partials                   ?        0
Impacted Files Coverage Δ
django_q/tests/settings.py 100% <100%> (ø)
django_q/tests/urls.py 100% <100%> (ø)
django_q/cluster.py 90.97% <100%> (ø)
django_q/signing.py 100% <100%> (ø)
django_q/tests/test_cluster.py 100% <100%> (ø)
django_q/tests/test_scheduler.py 100% <100%> (ø)
django_q/queues.py 100% <100%> (ø)
django_q/tasks.py 99.72% <100%> (ø)
django_q/tests/test_cached.py 100% <100%> (ø)
django_q/conf.py 71.17% <40%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 72b52b0...89cdf01. Read the comment docs.

@Koed00
Copy link
Owner

Koed00 commented Dec 12, 2017

Yeah, quite a lot of the tests are failing on Django 2.0, even though they ran fine in the alpha and beta stages.

I wish I had more time for it, but I just started a new job and I need to move by the end of the month.
Will try to find some time in the upcoming weekend.

@Eagllus
Copy link
Collaborator Author

Eagllus commented Dec 12, 2017

@Koed00 no worries I have time to figure this out!
I hope to get somewhere with the tests this week.

@Eagllus
Copy link
Collaborator Author

Eagllus commented Dec 12, 2017

ps: Enjoy your new job 😉

@Eagllus Eagllus closed this Dec 12, 2017
@Eagllus Eagllus reopened this Dec 12, 2017
@Eagllus
Copy link
Collaborator Author

Eagllus commented Dec 12, 2017

Closed it by mistake and it din't want me to open it but now it does XD

In Django 1.10 MIDDLEWARE_CLASSES is set to deprecated and is removed in Django 2.0.
The SessionAuthenticationMiddleware is also removed from Django 2.0 and was already
unconditionally enabled in Django 1.10.
This will not fix anything related to Django 2.0 but will allow MAC
users to run Django-Q allowing tests to be run locally.
@Eagllus
Copy link
Collaborator Author

Eagllus commented Dec 13, 2017

@Koed00 I'm kind of stuck here.

I found a way to fix the remaining problems (there all related to one thing)
The change that are made in django.core.signing and triggers effects that I can't seem to understand.

I added a commit showing the 'old' core.signing features working, hope you have time this weekend and that that it will save you some time.

By reverting django.core.signing to Django 1.11.8 version Django-Q works
This is not designed as a 'good' solution just to show where I think the
problem is.
@Eagllus
Copy link
Collaborator Author

Eagllus commented Jan 5, 2018

@Koed00 any update about the problem my pr was/is experiencing?

@Koed00
Copy link
Owner

Koed00 commented Jan 5, 2018

I've not had time to look into this yet. Just moved all my belongings into storage and found a temporary apartment. Making this a priority for the weekend. I need it to work reliably with Django 2 for a new project as well. Thanks for all the work you put in Ronald. Best wishes for the new year!

@Eagllus
Copy link
Collaborator Author

Eagllus commented Jan 5, 2018

@Koed00 no problem at all 😄
I use Django-Q daily in production so I don't mind working on it.
Best wishes to you for the new year, with a hope nice challenges 😉

@Koed00 Koed00 merged commit 004019b into Koed00:django_2_compat Jan 8, 2018
@Eagllus Eagllus deleted the django_2_compat branch January 9, 2018 08:57
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.

4 participants