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

task/WG-236: Websocket notifications #185

Merged
merged 8 commits into from
Apr 30, 2024

Conversation

tjgrafft
Copy link
Contributor

@tjgrafft tjgrafft commented Apr 3, 2024

Overview:

Backend changes for websocket notifications.

Related Jira tickets:

Summary of Changes:

  • Rather than using constant polling for notifications, I made changes to use a websocket solution. This PR/branch is to be used with the react front-end's branch that's named the same.
  • Added socket io package with poetry
  • In the app.py file added different events for socket to listen for, and respond appropriately.
  • Added CORS allowed origin for localhost since client and socket server are on different ports.
  • Changed number of workers for gunicorn from 4 to 1 bc documentation says that gunicorn only supports 1 worker when using websockets
  • Changed nginx.conf (local version) and its rules to handle socket.io path. This allowed for bi-directional handshake between client and server, and websocket connection to remain stable and continuously connected.

Testing Steps:

  1. See hazmapper PR branch with identical name for testing steps. Front-end

Notes:

  • This feature still needs wss security added before being deployed to prod. This can be done in nginx.conf file by adding the SSL cert and key that's related to prod VM (Nathan did this in PR WG-270, so most of the work that's left is on the front-end. However you'll need to set up nginx.conf to upgrade the http connection to handle websockets, I did this in the local version of nginx on this PR for reference). Then on the front-end change socket.io request (socketUtils file) from 'http://localhost:8888' to 'wss://hazmapper.tacc.utexas.edu' (or wherever nginx is running for prod env--might be prod.geoapi-services.tacc.utexas.edu? Note since we moved the port to 443 instead of 8888, we won't need to include the port bc wss defaults to 443).
  • Auth can also be changed, if desired. Right now the user must be logged in to get a token and thus establish a web socket connection with server. However, it's using the Bearer Token for auth/verification. If devs want to use JWT instead, you can use extraHeaders option in initial http request to set the JWT example code using extraHeaders

@tjgrafft tjgrafft changed the base branch from master to feature/websockets April 24, 2024 16:01
@tjgrafft tjgrafft marked this pull request as ready for review April 29, 2024 19:00
@tjgrafft tjgrafft requested a review from nathanfranklin April 29, 2024 20:18
Copy link
Collaborator

@nathanfranklin nathanfranklin left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Tested locally. works great. thanks for creating follow on task.

@tjgrafft tjgrafft merged commit 526f9f4 into feature/websockets Apr 30, 2024
3 checks passed
@tjgrafft tjgrafft deleted the task/WG-236-websocket-notifications branch April 30, 2024 16:04
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.

2 participants