Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#32128 closed Cleanup/optimization (fixed)

Issue with asgiref dependency installing Django 3.1.x

Reported by: Carlton Gibson Owned by: Carlton Gibson
Component: Core (Other) Version: 3.1
Severity: Release blocker Keywords:
Cc: Andrew Godwin, David Smith, Mariusz Felisiak Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

It looks like we've misspecified the asgiref dependency in Django 3.1.x.

Specifically, I think we wanted latest asgiref that wasn't 4 (i.e. a break) but installing we're not picking up the latest asgiref 3.3, which we want (because it makes the thread_sensitive parameter default True).

This came up on https://github.com/django/channels/pull/1522.

An example with a fresh venv:

(tmp-472bce83ec89b59) ~/ve/tmp-472bce83ec89b59 $ pip install asgiref
Collecting asgiref
  Using cached asgiref-3.3.0-py3-none-any.whl (19 kB)
Installing collected packages: asgiref
Successfully installed asgiref-3.3.0
(tmp-472bce83ec89b59) ~/ve/tmp-472bce83ec89b59 $ pip install Django
Collecting Django
  Using cached Django-3.1.2-py3-none-any.whl (7.8 MB)
Collecting asgiref~=3.2.10
  Using cached asgiref-3.2.10-py3-none-any.whl (19 kB)
Collecting pytz
  Using cached pytz-2020.1-py2.py3-none-any.whl (510 kB)
Collecting sqlparse>=0.2.2
  Using cached sqlparse-0.4.1-py3-none-any.whl (42 kB)
Installing collected packages: asgiref, pytz, sqlparse, Django
  Attempting uninstall: asgiref
    Found existing installation: asgiref 3.3.0
    Uninstalling asgiref-3.3.0:
      Successfully uninstalled asgiref-3.3.0
Successfully installed Django-3.1.2 asgiref-3.2.10 pytz-2020.1 sqlparse-0.4.1
(tmp-472bce83ec89b59) ~/ve/tmp-472bce83ec89b59 $ pip install -U asgiref
Collecting asgiref
  Using cached asgiref-3.3.0-py3-none-any.whl (19 kB)
Installing collected packages: asgiref
  Attempting uninstall: asgiref
    Found existing installation: asgiref 3.2.10
    Uninstalling asgiref-3.2.10:
      Successfully uninstalled asgiref-3.2.10
ERROR: After October 2020 you may experience errors when installing or updating packages. This is because pip will change the way that it resolves dependency conflicts.

We recommend you use --use-feature=2020-resolver to test your packages with the new resolver before it becomes the default.

django 3.1.2 requires asgiref~=3.2.10, but you'll have asgiref 3.3.0 which is incompatible.
Successfully installed asgiref-3.3.0

  • This is using the old resolver but the new one will pick asgiref 3.2.10 too.
  • I installed asgiref first here just for demonstration. A straight pip install Django equally picks 3.2.10.

I think this is a RB as when I pip install Django 3.1 I want (need) the latest asgiref, but hoping for opinions on that.

Change History (9)

in reply to:  description comment:1 by Mariusz Felisiak, 3 years ago

Type: BugCleanup/optimization

Replying to Carlton Gibson:

It looks like we've misspecified the asgiref dependency in Django 3.1.x.

Specifically, I think we wanted latest asgiref that wasn't 4 (i.e. a break) but installing we're not picking up the latest asgiref 3.3, which we want (because it makes the thread_sensitive parameter default True).

I don't agree, we did this on purpose to avoid any breaking changes in supported versions of asgiref, from IRC (June 2020):

<andrewgodwin> Any opinions on changing the Django asgiref dependency to `~=3.2,>=3.2.8`? I'd like to allow a 4.0 release to possibly exist with different APIs, and 3.2.8 has an important contextvars fix in it.
<felixx> andrewgodwin: I'm not sure if we need to add >= 3.2.8
<andrewgodwin> felixx: Well, if people already have an environment with 3.2.7 installed and Django 3.0, I sort of want to force an upgrade there
<felixx> we officially always recommend the newest version
<felixx> we don't have an asgiref version pinned in Django 3.1
<felixx> we can always pin it before the rc1
<andrewgodwin> Yeah, my main concern is moving from `>=3.2` to `~=3.2`
<andrewgodwin> It sort of blocks us from making a breaking change in asgiref for 3 years
<andrewgodwin> The Sentry folks would say it's important enough (3.2.7 doesn't handle contextvars correctly)
<felixx> in Django 3.0 we have ~3.2
<andrewgodwin> Yes, I saw that
<untitaker> hi I just got pinged by keyword. I'm the one from Sentry making that claim. I don't really know anything about the tradeoff you are considering, but my concern is that when somebody upgrades django without upgrading asgiref, they will see their before_request/after_request/got_request_exception being executed with the wrong context. Whether that's within scope of the API contract (and whether that is a regression)
<untitaker> is up to you.
<felixx> IMO we shouldn't change it
<felixx> we can discuss what to do with Django 3.1
<andrewgodwin> Yeah I care about Django 3.1, not 3.0
<andrewgodwin> 3.0 doesn't have proper async views - as untitaker says, that will be much more confusing in 3.1 when people start using them
<felixx> I will discuss this tomorrow with Carlton, I asked him about pinning asgiref version for Django 3.1 sometime ago
<andrewgodwin> OK!

As far as I'm aware asgiref 3.3 is a breaking release because it changed the default value of thread_sensitive. We can of course change the requirements, but IMO that's a new feature (or cleanup) but not a bug. It will probably require some docs changes.

comment:2 by Carlton Gibson, 3 years ago

I think we made a mistake. As the settings are users can't opt-in to the version they really should be on, without knowing that they really do want to ignore the scary warnings from pip. We've made the ~= too tight. Let's not back-and-forth on trac flags, but I think we need to fix this. 😬

comment:3 by Mariusz Felisiak, 3 years ago

Triage Stage: UnreviewedAccepted

Tentatively accepted 😉, I would like to see docs changes (e.g. docs/topics/async.txt) as a part of this patch.

comment:4 by Andrew Godwin, 3 years ago

I did partially introduce the version bump in asgiref because I didn't want existing Django installs to just pull it in; I think incrementing the dependency in a patch release and updating the docs would be a good move here.

comment:5 by Carlton Gibson, 3 years ago

Thanks both. We should be able to adjust the requirement so it:

  • Will already be satisfied if you have asgiref installed, but
  • will let you update (without the scary warnings) if you $ pip install -U asgiref

Time to have a play and see what works 😀

comment:6 by Carlton Gibson, 3 years ago

Owner: changed from nobody to Carlton Gibson
Status: newassigned

comment:7 by Mariusz Felisiak, 3 years ago

Has patch: set
Triage Stage: AcceptedReady for checkin

comment:8 by Carlton Gibson <carlton@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In e17ee446:

Fixed #32128 -- Added asgiref 3.3 compatibility.

Thread sensitive parameter is True by default from asgiref v3.3.0.
Added an explicit thread_sensitive=False to previously implicit uses.

comment:9 by Carlton Gibson <carlton.gibson@…>, 3 years ago

In d00127c:

[3.1.x] Fixed #32128 -- Added asgiref 3.3 compatibility.

Thread sensitive parameter is True by default from asgiref v3.3.0.
Added an explicit thread_sensitive=False to previously implicit uses.

Backport of e17ee4468875077b90b70bb6a589ebad7493f757 from master

Note: See TracTickets for help on using tickets.
Back to Top