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

Reimplement async support #209

Merged
merged 6 commits into from Mar 23, 2023
Merged

Reimplement async support #209

merged 6 commits into from Mar 23, 2023

Conversation

jrobichaud
Copy link
Owner

@jrobichaud jrobichaud commented Mar 23, 2023

  • RequestMiddleware now properly handles sync/async requests
  • Remove legacy exception handling code from older implementation

@codecov
Copy link

codecov bot commented Mar 23, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (e48bb06) 100.00% compared to head (04040ef) 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##            master      #209    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           41        41            
  Lines         1272      1104   -168     
==========================================
- Hits          1272      1104   -168     
Impacted Files Coverage Δ
django_structlog/signals.py 100.00% <ø> (ø)
django_structlog/__init__.py 100.00% <100.00%> (ø)
django_structlog/celery/middlewares.py 100.00% <100.00%> (ø)
django_structlog/middlewares/request.py 100.00% <100.00%> (ø)
test_app/tests/middlewares/test_celery.py 100.00% <100.00%> (ø)
test_app/tests/middlewares/test_request.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jrobichaud jrobichaud self-assigned this Mar 23, 2023
@jrobichaud jrobichaud added the enhancement New feature or request label Mar 23, 2023
@jrobichaud jrobichaud merged commit eb7e456 into master Mar 23, 2023
12 checks passed
@jrobichaud jrobichaud deleted the fix-async branch March 23, 2023 19:13
@adinsoon
Copy link

adinsoon commented Mar 24, 2023

I tried to run 5.0.0 at us, we have python 3.9 and django 3.2.15
By default we have asgiref==3.5.0

  1. When we have asgiref==3.5.0 and django-structlog==4.1.1 everything works as before
  2. When we have asgiref==3.5.0 and django-structlog==5.0.0 we get ImportError and error on container:
web-1  |   File "/usr/local/lib/python3.9/dist-packages/django_structlog/middlewares/__init__.py", line 1, in <module>
web-1  |     from .request import RequestMiddleware, request_middleware_router  # noqa F401
web-1  |   File "/usr/local/lib/python3.9/dist-packages/django_structlog/middlewares/request.py", line 4, in <module>
web-1  |     from asgiref.sync import iscoroutinefunction, markcoroutinefunction
web-1  | ImportError: cannot import name 'iscoroutinefunction' from 'asgiref.sync' (/usr/local/lib/python3.9/dist-packages/asgiref/sync.py)
  1. When we have asgiref==3.6.0 and django-structlog==5.0.0. we still got ImportError as above
  2. When we have asgiref==3.6.0 and django-structlog==4.1.1 it works as before, but at least I can import iscoroutinefunction, markcoroutinefunction on my own

It looks like it may be dependency issue for Python 3.9 (at least) in the library. Moreover, when bumping django-structlog to 5.0.0 and generating requirements, asgiref doesn't bump itself to 3.6.0 but stays at 3.5.0. Looks like django-structlog in 5.0.0 still uses asgiref==3.5.0 since iscoroutinefunction and markcoroutinefunction are available from 3.6.0 IIUC

Do you have any idea why it could work like that?

@jrobichaud
Copy link
Owner Author

jrobichaud commented Mar 24, 2023

I did not noticed it was recently added in asgiref.

The problem is in django-structlog's pyproject.toml line 19.

I am not specifying a minimum version for asgiref.

@adinsoon
Copy link

Do you have an idea how to deal with it? I understand that it should work if we manually install version 3.6 of asgiref, however it doesn't work with django-structlog version 5.0.0

@jrobichaud
Copy link
Owner Author

What does not work with django-structlog?

@adinsoon
Copy link

adinsoon commented Mar 24, 2023

Looks like I've fix it right now.
I explicitly specified the asgiref version to 3.6.0 in requirements.in, then bumped it, then specified django-structlog version to 5.0.0 and then bumped again. ImportError disappeared :)

@jrobichaud
Copy link
Owner Author

I am currently working on 5.0.1 that will explicitly fix this issue.

@jrobichaud
Copy link
Owner Author

@adinsoon see #212

@adinsoon
Copy link

@jrobichaud Seems to be in order. Thank you once again!

@jrobichaud
Copy link
Owner Author

@adinsoon

I confirm #212 and 5.0.1 fixes the problem. Here's a proof.

I first install django-structlog==5.0.0 asgiref==3.5.0:

$ python3 -m venv ./venv
$ source ./venv/bin/activate
$ pip install django-structlog==5.0.0 asgiref==3.5.0
Collecting django-structlog==5.0.0
  Downloading django_structlog-5.0.0-py3-none-any.whl (13 kB)
Collecting asgiref==3.5.0
  Downloading asgiref-3.5.0-py3-none-any.whl (22 kB)
Collecting structlog>=21.4.0
  Using cached structlog-22.3.0-py3-none-any.whl (61 kB)
Collecting django-ipware
  Using cached django_ipware-5.0.0-py2.py3-none-any.whl (9.4 kB)
Collecting django>=3.2
  Using cached Django-3.2.18-py3-none-any.whl (7.9 MB)
Collecting typing-extensions; python_version < "3.8"
  Using cached typing_extensions-4.5.0-py3-none-any.whl (27 kB)
Collecting importlib-metadata; python_version < "3.8"
  Using cached importlib_metadata-6.1.0-py3-none-any.whl (21 kB)
Collecting pytz
  Using cached pytz-2022.7.1-py2.py3-none-any.whl (499 kB)
Collecting sqlparse>=0.2.2
  Using cached sqlparse-0.4.3-py3-none-any.whl (42 kB)
Collecting zipp>=0.5
  Using cached zipp-3.15.0-py3-none-any.whl (6.8 kB)
Installing collected packages: typing-extensions, zipp, importlib-metadata, structlog, asgiref, django-ipware, pytz, sqlparse, django, django-structlog
Successfully installed asgiref-3.5.0 django-3.2.18 django-ipware-5.0.0 django-structlog-5.0.0 importlib-metadata-6.1.0 pytz-2022.7.1 sqlparse-0.4.3 structlog-22.3.0 typing-extensions-4.5.0 zipp-3.15.0

Then explicitly install only django-structlog==5.0.1.

$ pip install django-structlog==5.0.1
Collecting django-structlog==5.0.1
  Downloading django_structlog-5.0.1-py3-none-any.whl (13 kB)
Requirement already satisfied: django-ipware in ./venv/lib/python3.7/site-packages (from django-structlog==5.0.1) (5.0.0)
Collecting asgiref>=3.6.0
  Using cached asgiref-3.6.0-py3-none-any.whl (23 kB)
Requirement already satisfied: django>=3.2 in ./venv/lib/python3.7/site-packages (from django-structlog==5.0.1) (3.2.18)
Requirement already satisfied: structlog>=21.4.0 in ./venv/lib/python3.7/site-packages (from django-structlog==5.0.1) (22.3.0)
Requirement already satisfied: typing-extensions; python_version < "3.8" in ./venv/lib/python3.7/site-packages (from asgiref>=3.6.0->django-structlog==5.0.1) (4.5.0)
Requirement already satisfied: sqlparse>=0.2.2 in ./venv/lib/python3.7/site-packages (from django>=3.2->django-structlog==5.0.1) (0.4.3)
Requirement already satisfied: pytz in ./venv/lib/python3.7/site-packages (from django>=3.2->django-structlog==5.0.1) (2022.7.1)
Requirement already satisfied: importlib-metadata; python_version < "3.8" in ./venv/lib/python3.7/site-packages (from structlog>=21.4.0->django-structlog==5.0.1) (6.1.0)
Requirement already satisfied: zipp>=0.5 in ./venv/lib/python3.7/site-packages (from importlib-metadata; python_version < "3.8"->structlog>=21.4.0->django-structlog==5.0.1) (3.15.0)
Installing collected packages: asgiref, django-structlog
  Attempting uninstall: asgiref
    Found existing installation: asgiref 3.5.0
    Uninstalling asgiref-3.5.0:
      Successfully uninstalled asgiref-3.5.0
  Attempting uninstall: django-structlog
    Found existing installation: django-structlog 5.0.0
    Uninstalling django-structlog-5.0.0:
      Successfully uninstalled django-structlog-5.0.0
Successfully installed asgiref-3.6.0 django-structlog-5.0.1

asgiref is correctly updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants