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

Move websockets-stuff to spa-dir #1175

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Move websockets-stuff to spa-dir #1175

wants to merge 1 commit into from

Conversation

hmpf
Copy link
Contributor

@hmpf hmpf commented Feb 14, 2025

Replaces #785

This renames the "argus.ws" module to "argus.spa.ws" and turns "argus.spa" into an app. This will make it even easier to drop support for spa and redis when the day comes.

argus.ws.asgi has been renamed to argus.spa.ws.asgi.

Redis issues: #253, #1063

Copy link

Test results

   10 files  1 060 suites   37m 31s ⏱️
  536 tests   535 ✅  1 💤 0 ❌
5 360 runs  5 350 ✅ 10 💤 0 ❌

Results for commit 38aa3d7.

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 16.66667% with 5 lines in your changes missing coverage. Please review.

Project coverage is 77.93%. Comparing base (9f6df5d) to head (38aa3d7).

Files with missing lines Patch % Lines
src/argus/spa/spa_settings.py 0.00% 4 Missing ⚠️
src/argus/spa/settings.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1175      +/-   ##
==========================================
- Coverage   78.48%   77.93%   -0.55%     
==========================================
  Files         141      141              
  Lines        5442     5447       +5     
==========================================
- Hits         4271     4245      -26     
- Misses       1171     1202      +31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@elfjes elfjes left a comment

Choose a reason for hiding this comment

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

I am mostly fine with this MR. I would however, keep the ASGI_APPLICATION in the base settings. Users can choose wether they want wsgi or asgi by selecting a webserver or (gunicorn) worker that does one or the other. No need to hide the ASGI_APPLICATION setting the spa settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Changes requested
Development

Successfully merging this pull request may close these issues.

3 participants