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

Drop Django 1.10 support #5657

Merged
merged 2 commits into from Jul 6, 2018
Merged

Conversation

rpkilby
Copy link
Member

@rpkilby rpkilby commented Dec 5, 2017

Per the official recommendation, we should drop 1.10 support now that 2.0 has been released.

@rpkilby rpkilby added this to the v3.8 milestone Dec 5, 2017
@xordoquy
Copy link
Collaborator

xordoquy commented Dec 5, 2017

I'm not so much in a hurry to drop this.
Moreover, we should make sure this won't land in any 3.7.x

@rpkilby
Copy link
Member Author

rpkilby commented Dec 5, 2017

Moreover, we should make sure this won't land in any 3.7.x

Yep, added to the 3.8 milestone.

@tomchristie
Copy link
Member

tomchristie commented Dec 5, 2017

I'm not sure I'm convinced by Django's recommendations re. "Third-party library support for older version of Django" - Dropping 1.10 already seems unnecessarily premature, since it was the latest version of Django just 10 months ago.

To my mind a policy of "support the latest 3 versions" would be more preferable to aligning with Django's current recommendations.

@rpkilby
Copy link
Member Author

rpkilby commented Dec 5, 2017

That's fair. I'll revisit this once Django 2.1 is out.

@rpkilby rpkilby closed this Dec 5, 2017
@rpkilby rpkilby removed this from the v3.8 milestone Dec 5, 2017
@carltongibson
Copy link
Collaborator

carltongibson commented Dec 5, 2017

... "support the latest 3 versions"...

Back to the Old School. 🙂

That was how we used to do it. There's something in it.

However... 🙂

  1. The issue with just 10 months ago is covered by Django already with the LTS versions. If you're not prepared to keep up, use the LTS versions, because...
  2. Once a version is EOL it doesn't even get security fixes. It's not safe to use.

On 2: it's not a theoretical risk. With 10 minutes of Google, you can download automated scripts that will try every known exploit against a server. Bots run these kits against every addressable server on the internet. (Look at your server logs. You can see it in action. All the time.)

If you run insecure software, you will be hacked. When. Not if.

So for us,

  1. We're sending the wrong signal by not following Django's Supported Versions. We're implicitly saying it's OK to use EOL software. It's not. (Users will delay updating Django until the app they use drops their current version.)
  2. Dropping versions makes our lives easier. Look at the compat changes in since we dropped 1.8/1.9. Look at Add HStoreField, postgres fields tests #5654: we can straight fix it, or introduce a compat shim to support an EOL version. Etc.

It used to be that upgrading Django was hard. It's not so any more.

Also, it's not like 3.7.x will stop working. If you're running a service and don't want to upgrade Django, just don't upgrade DRF either.

I'm +1 for dropping 1.10.

@tomchristie
Copy link
Member

tomchristie commented Dec 5, 2017

Once a version is EOL it doesn't even get security fixes. It's not safe to use.

Except "It's not safe to use." isn't necessarily true. It's perfectly reasonable to assess security issues that come up and determine (1) if they affect your particular service (2) if you want to resolve the issue locally.

Either (1) or (2) will often be the case.

I'd agree wrt. to #5654 tho - if dropping support is something that makes our lives easier, then there's a valid argument for it. (Tho we could equally well exclude the tests for 1.10 in that case)

@tomchristie
Copy link
Member

tomchristie commented Dec 5, 2017

If you're running a service and don't want to upgrade Django, just don't upgrade DRF either.

That's why I'm of a different opinion tho - I think there are a lot of users who stay up to date with REST framework more frequently than they'll choose to stay up to date with Django, which is a perfectly reasonable business decision, if upgrading Django is more impactful than upgrading REST framework (generally the case)

I'm not going to say I'm 100% one way or the other, but I'm not convinced that dropped 1.10 already is necessarily in our users interests.

@xordoquy
Copy link
Collaborator

xordoquy commented Dec 5, 2017

The key point here is that you have much more dependancies on Django than you have on DRF. Some even take a long time (if ever) to upgrade which makes Django upgrades much slower than it should be.

@carltongibson
Copy link
Collaborator

carltongibson commented Dec 5, 2017

I think we have different user avatars. 😃

"Running an EOL + Doing Due-Dilligence on emerging security issues" — Not any project I ever saw. 😃

I take @xordoquy's point. I guess I'd point people who can't keep Django updated to the LTS (although since 1.8, keeping updated is much easier than it ever was)

For me it's about signalling: our users (on my avatar) look to what we support as a guide for what they should do. For me we're sending the wrong signal if we don't follow Django's Supported Versions.

However I'm on holiday. I leave it with you. 😉

@tomchristie
Copy link
Member

tomchristie commented Dec 5, 2017

Going to consider this an open question. For review once we're getting closer to 3.8, right?
Thanks so much for the work on it @rpkilby! :)

@rpkilby
Copy link
Member Author

rpkilby commented Dec 21, 2017

Sorry - temporarily borked the PR and had to fix it.

For review once we're getting closer to 3.8, right?

Given that 3.7.x is pretty much done, I assume we're re-reviewing this?

@rpkilby rpkilby added this to the 3.8 Release milestone Dec 21, 2017
@carltongibson
Copy link
Collaborator

carltongibson commented Dec 22, 2017

Lets chat about it in January.

@jdufresne
Copy link
Contributor

jdufresne commented Jan 7, 2018

You can drop the trove classifier from setup.py in this change, 'Framework :: Django :: 1.10',.

./rest_framework/urls.py has some potential cleanup:

if django.VERSION < (1, 11):
    login = views.login
    login_kwargs = {'template_name': 'rest_framework/login.html'}
    logout = views.logout
else:
    login = views.LoginView.as_view(template_name='rest_framework/login.html')
    login_kwargs = {}
    logout = views.LogoutView.as_view()

as does compat.py:

def authenticate(request=None, **credentials):
    from django.contrib.auth import authenticate
    if django.VERSION < (1, 11):
        return authenticate(**credentials)
    else:
        return authenticate(request=request, **credentials)

@rpkilby rpkilby force-pushed the drop-django110 branch 2 times, most recently from 0365d79 to 1ff0b16 Compare Jan 8, 2018
@carltongibson carltongibson removed this from the 3.8 Release milestone Jan 25, 2018
@carltongibson
Copy link
Collaborator

carltongibson commented Jan 25, 2018

I'm going to de-milestone for now.

Lets see if we can roll v3.8 keeping Django 1.10. (If there's a blocker we can look again at that.)

I'd then look to drop 1.10 for v3.9, when we get to that.

@rpkilby rpkilby added this to the 3.9 Release milestone Jun 23, 2018
@rpkilby
Copy link
Member Author

rpkilby commented Jun 23, 2018

Adding this to the 3.9 milestone, given that it's going to be upon us fairly soon.

@carltongibson
Copy link
Collaborator

carltongibson commented Jul 2, 2018

OK, we're green on this. @rpkilby I saw you updated: anything else need doing, or you happy for a review? (Ta!)

@rpkilby
Copy link
Member Author

rpkilby commented Jul 2, 2018

Should be good for review. Are we rolling 3.8.3 first?

@carltongibson
Copy link
Collaborator

carltongibson commented Jul 2, 2018

Are we rolling 3.8.3 first?

Could do. Wasn't planning on it. (Thoughts welcome.)

@rpkilby
Copy link
Member Author

rpkilby commented Jul 2, 2018

There's a 3.8.3 milestone. Were you planning on dropping that and rolling 3.9 instead, or are you planning on just merging this into 3.8.3?

If it's the latter, I don't think we should drop support for Django 1.10 on a patch release.

@carltongibson
Copy link
Collaborator

carltongibson commented Jul 2, 2018

IF we got to 3.8.3 then the merged tickets would be on that milestone. IF we just go for 3.9 we just adjust the milestone. (Was the idea.)

I CAN roll a 3.8.3 if there's a reason. Otherwise all steam ahead for 3.9.

@rpkilby
Copy link
Member Author

rpkilby commented Jul 2, 2018

I CAN roll a 3.8.3 if there's a reason. Otherwise all steam ahead for 3.9.

I don't know if there's a reason to cut a 3.8.3 release, except maybe to cut down on noise for the 3.9 release. Most of 3.8.3 is docs-related.

I'm fine with just moving ahead with 3.9 - it's up to you 😄.

@carltongibson carltongibson merged commit a628a2d into encode:master Jul 6, 2018
1 check passed
@rpkilby rpkilby deleted the drop-django110 branch Jul 6, 2018
pchiquet pushed a commit to pchiquet/django-rest-framework that referenced this pull request Nov 17, 2020
* Remove Django 1.10 from CI

* Remove Django 1.10 compat code
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

5 participants