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

[feature] Upgrade to channels 4.x #332

Closed
wants to merge 3 commits into from

Conversation

AviGawande
Copy link

@AviGawande AviGawande commented Feb 4, 2025

  • Updated websocket consumer to use AsyncWebsocketConsumer
  • Updated routing to use re_path
  • Updated handlers to use async/await pattern
  • Updated channels dependency to >=4.0.0,<5.0.0

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • I have updated the documentation.

Reference to Existing Issue = #388

Fixes centralize-django-channels

Please open a new issue if there isn't an existing issue yet.

Description of Changes

Please describe these changes.

Screenshot

Please include any relevant screenshots.

- Updated websocket consumer to use AsyncWebsocketConsumer
- Updated routing to use re_path
- Updated handlers to use async/await pattern
- Updated channels dependency to >=4.0.0,<5.0.0
Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The build is failing, please run the sample app tests locally with the following command to see if you can replicate:

SAMPLE_APP=1 ./runtests.py

Copy link
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue still persists.
I was able to replicate the issue with these commands:

mkvirtualenv --python=python3 channels4
pip install -e . && pip install -r requirements-test.txt
SAMPLE_APP=1 ./runtests.py

Test output:

======================================================================
ERROR: openwisp2.sample_notifications.tests.test_websockets (unittest.loader._FailedTest)
----------------------------------------------------------------------
ImportError: Failed to import test module: openwisp2.sample_notifications.tests.test_websockets
Traceback (most recent call last):
  File "/usr/lib/python3.8/unittest/loader.py", line 436, in _find_test_path
    module = self._get_module_from_name(name)
  File "/usr/lib/python3.8/unittest/loader.py", line 377, in _get_module_from_name
    __import__(name)
  File "/home/nemesis/Code/openwisp/openwisp-notifications/tests/openwisp2/sample_notifications/tests/test_websockets.py", line 1, in <module>
    from openwisp_notifications.tests.test_websockets import (
  File "/home/nemesis/Code/openwisp/openwisp-notifications/openwisp_notifications/tests/test_websockets.py", line 9, in <module>
    from channels.testing import WebsocketCommunicator
  File "/home/nemesis/.virtualenvs/channels4/lib/python3.8/site-packages/channels/testing/__init__.py", line 3, in <module>
    from .live import ChannelsLiveServerTestCase  # noqa
  File "/home/nemesis/.virtualenvs/channels4/lib/python3.8/site-packages/channels/testing/live.py", line 3, in <module>
    from daphne.testing import DaphneProcess
ModuleNotFoundError: No module named 'daphne'

Are you able to replicate this issue locally @AviGawande?

@AviGawande
Copy link
Author

AviGawande commented Feb 7, 2025

@nemesifier I tried to replicate the issue but i am getting different error.

======================================================================
ERROR: openwisp2.sample_notifications.tests.test_websockets (unittest.loader._FailedTest)
----------------------------------------------------------------------
ImportError: Failed to import test module: openwisp2.sample_notifications.tests.test_websockets
Traceback (most recent call last):
  File "/usr/lib/python3.10/unittest/loader.py", line 436, in _find_test_path
    module = self._get_module_from_name(name)
  File "/usr/lib/python3.10/unittest/loader.py", line 377, in _get_module_from_name
    __import__(name)
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/tests/openwisp2/sample_notifications/tests/test_websockets.py", line 1, in <module>
    from openwisp_notifications.tests.test_websockets import (
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/openwisp_notifications/tests/test_websockets.py", line 76, in <module>
    class TestNotificationSockets:
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/openwisp_notifications/tests/test_websockets.py", line 77, in TestNotificationSockets
    application = import_string(getattr(settings, 'ASGI_APPLICATION'))
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/env/lib/python3.10/site-packages/django/utils/module_loading.py", line 30, in import_string
    return cached_import(module_path, class_name)
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/env/lib/python3.10/site-packages/django/utils/module_loading.py", line 15, in cached_import
    module = import_module(module_path)
  File "/usr/lib/python3.10/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/tests/openwisp2/asgi.py", line 23, in <module>
    AuthMiddlewareStack(URLRouter(get_routes(consumers)))
TypeError: get_routes() takes 0 positional arguments but 1 was given

I tried changes locally but it didn't worked for notification_update_handler

@nemesifier
Copy link
Member

@AviGawande the fact that you get a different error means that probably you're not executing the same steps of the CI build.
Can you look at the steps exeucted by CI build please?
https://github.com/openwisp/openwisp-notifications/blob/master/.github/workflows/build.yml
Ensure you're creating a new python virtualenvironment and executing the same steps, then run:

SAMPLE_APP=1 ./runtests.py.

@AviGawande
Copy link
Author

@nemesifier I followed the same steps you mentioned but still got this error.

(env) abhigawande@DESKTOP-K9K53L4:~/projects/openwisp/notification/openwisp-notifications$ ./runtests.py
Traceback (most recent call last):
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/./runtests.py", line 22, in <module>
    execute_from_command_line(args)
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/env/lib/python3.10/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
    utility.execute()
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/env/lib/python3.10/site-packages/django/core/management/__init__.py", line 416, in execute
    django.setup()
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/env/lib/python3.10/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/env/lib/python3.10/site-packages/django/apps/registry.py", line 116, in populate
    app_config.import_models()
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/env/lib/python3.10/site-packages/django/apps/config.py", line 269, in import_models
    self.models_module = import_module(models_module_name)
  File "/usr/lib/python3.10/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1050, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1027, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1006, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 688, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 883, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/env/lib/python3.10/site-packages/openwisp_users/models.py", line 10, in <module>
    from .base.models import (
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/env/lib/python3.10/site-packages/openwisp_users/base/models.py", line 47, in <module>
    class AbstractUser(BaseUser):
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/env/lib/python3.10/site-packages/django/db/models/base.py", line 143, in __new__
    new_class.add_to_class("_meta", Options(meta, app_label))
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/env/lib/python3.10/site-packages/django/db/models/base.py", line 371, in add_to_class
    value.contribute_to_class(cls, name)
  File "/home/abhigawande/projects/openwisp/notification/openwisp-notifications/env/lib/python3.10/site-packages/django/db/models/options.py", line 220, in contribute_to_class
    raise TypeError(
TypeError: 'class Meta' got invalid attribute(s): index_together
(env) abhigawande@DESKTOP-K9K53L4:~/projects/openwisp/notification/openwisp-notifications$ 

@nemesifier
Copy link
Member

'class Meta' got invalid attribute(s): index_together

@AviGawande please enable your python virtualenv and tell me what's the output of the following command?

pip freeze | grep -i "Django=="

@AviGawande
Copy link
Author

AviGawande commented Feb 11, 2025

@nemesifier my venv is activated but still this issue persists.
Output:

(env) abhigawande@DESKTOP-K9K53L4:~/projects/openwisp/notification/openwisp-notifications$ pip freeze | grep -i "Django=="
Django==3.2.25

@nemesifier
Copy link
Member

@AviGawande make sure you you have Django 4.2, not sure channels 4 supports Django 3.2 (please check). Make also sure your openwisp-notifications fork is installed in your python virtual env.

@AviGawande
Copy link
Author

@nemesifier Sure,I am thinking of brute forcing this issue ,means i will close this PR and test and build the changes locally and will then Raise PR.

@AviGawande AviGawande closed this by deleting the head repository Feb 12, 2025
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.

[feature] Centralize django-channels dependency and upgrade to latest version
2 participants