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

Add temporary asgiref requirement #1261

Merged
merged 1 commit into from
Sep 26, 2023
Merged

Add temporary asgiref requirement #1261

merged 1 commit into from
Sep 26, 2023

Conversation

ddabble
Copy link
Member

@ddabble ddabble commented Sep 25, 2023

Description

#1209 added an implicit dependency on asgiref>=3.6, as the imported function iscoroutinefunction() was introduced in asgiref 3.6.0 (see asgiref's changelog), but a lower version is the minimum requirement of Django 4.1 and earlier (see e.g. Django 4.1.11's setup.cfg).

This means that users who happen to install django-simple-history together with Django 4.1 or lower, might implicitly also install a lower version of asgiref than what's actually required to run our code.

The easiest solution to this seems to be adding a requirement for asgiref>=3.6, which can be removed when our minimum required Django version is 4.2 or higher.

Related Issue

Closes #1255.

Motivation and Context

See the discussion of the issue linked above.

How Has This Been Tested?

Through GitHub actions.

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
    • This seems unlikely, however, as I'm assuming that most users of this library simply rely on the same version of asgiref as is installed with the Django version they're using

Checklist:

  • I have run the pre-commit run command to format and lint.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have added my name and/or github handle to AUTHORS.rst
  • I have added my change to CHANGES.rst
  • All new and existing tests passed.

See #1255
for discussion.

Link to Django 4.1's required `asgiref` version:
https://github.com/django/django/blob/4.1.11/setup.cfg#L42
Link to Django 4.2's required `asgiref` version:
https://github.com/django/django/blob/4.2/setup.cfg#L42
@ddabble ddabble requested a review from valberg September 25, 2023 23:12
@codecov
Copy link

codecov bot commented Sep 25, 2023

Codecov Report

Merging #1261 (d1ce47a) into master (329659b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1261   +/-   ##
=======================================
  Coverage   96.94%   96.94%           
=======================================
  Files          23       23           
  Lines        1277     1277           
  Branches      210      210           
=======================================
  Hits         1238     1238           
  Misses         21       21           
  Partials       18       18           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ddabble ddabble merged commit fd9aa52 into master Sep 26, 2023
36 checks passed
@ddabble ddabble deleted the fix/pin-asgiref branch September 26, 2023 11:02
@amandeep-r
Copy link

Would it be possible to get a release out including this change?

@ddabble
Copy link
Member Author

ddabble commented Feb 17, 2024

Sure, working on a release PR (#1290) right now 🙂

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.

Commit 61df0e2f5e90 introduced a requirement for asgiref 3.6.0+
3 participants