-
Notifications
You must be signed in to change notification settings - Fork 113
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
refactor code using black formatter and add dockerhub push cmd, and update base image to version containing cudnn8 #13
base: main
Are you sure you want to change the base?
Conversation
feat: docker hub push feat: update websocket server to aiohttp server with include client
Hey there could you explain the rationale behind these changes? There are a lot of files affected including beautification |
nvidia/cuda:11.6.2-base-ubuntu20.04 does not include cudnn, so users need to install cudnn and other required dependencies. |
@@ -1,5 +1,5 @@ | |||
# Use an NVIDIA CUDA base image with Python 3 | |||
FROM nvidia/cuda:11.6.2-base-ubuntu20.04 | |||
FROM nvidia/cuda:11.6.2-cudnn8-runtime-ubuntu20.04 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Dockerfile in the current repository will lead to the following issues:
Client 583ffd78-2bfe-4e1e-bbbc-8efed00e1904 connected
2024-05-20T08:38:53.837582891Z Could not load library libcudnn_ops_infer.so.8. Error: libcudnn_ops_infer.so.8: cannot open shared object file: No such file or directory
The current commit can fix this issue.
This PR contains too many changes. I will create related issues and rewrite the relevant commits. |
continue | ||
else: | ||
print(f"Unexpected message type from {client.client_id}") | ||
def setup_routes(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is redeclared.
@jinmiaoluo is this PR still relevant afer merging the other one? |
@alesaccoia This pull request contains several useful changes. We should separate and merge them individually, focusing on the following topics:
|
@alesaccoia The code for the first and third topic has been rewritten in PR: #21 , please take a look. |
No description provided.