-
Notifications
You must be signed in to change notification settings - Fork 55
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
Allow running on remote orchestrator #80
Conversation
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.
Looks good to me, though hard coding all the requirements in the Docker settings feels a bit ugly. Suggested referencing the file instead.
requirements=[ | ||
"langchain==0.0.263", | ||
"openai==0.27.2", | ||
"slack-bolt==1.16.2", | ||
"slack-sdk==3.20.0", | ||
"fastapi", | ||
"flask", | ||
"uvicorn", | ||
"gcsfs==2023.5.0", | ||
"faiss-cpu==1.7.3", | ||
"unstructured==0.5.7", | ||
"tiktoken", | ||
"bs4" | ||
], |
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.
I couldn't tell if this is different from the fixed files themselves, but would it be possible instead to just specify the requirements file itself? Seems safer, given that these files do actually get updated fairly often.
docker_settings = DockerSettings(requirements="/path/to/requirements.txt")
For demo's I like running on vertex, so some requirements are needed in teh docker image. Also the open_api token is needed