-
Notifications
You must be signed in to change notification settings - Fork 269
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
Emrgnt cmplxty patch 2 #255
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
❌ Changes requested.
- Reviewed the entire pull request up to e63ac47
- Looked at
41
lines of code in2
files - Took 55 seconds to review
More info
- Skipped
0
files when reviewing. - Skipped posting
0
additional comments because they didn't meet confidence threshold of50%
.
Workflow ID: wflow_jvmEZeFb6y8zmgC0
Want Ellipsis to fix these issues? Tag @ellipsis-dev
in a comment. We'll respond in a few minutes. Learn more here.
CMD ["uvicorn", "r2r.examples.basic.app:app", "--host", "0.0.0.0", "--port", "8000", "--workers", "8"] |
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.
Consider making the number of workers configurable through an environment variable instead of hardcoding it to '8'. This would allow for more flexibility depending on the server resources available.
sess.execute(text("create schema if not exists vecs;")) | ||
except Exception as e: | ||
if "already exists" not in str(e): | ||
raise ValueError( |
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.
Consider raising a more specific error, such as a DatabaseError, instead of a ValueError. This would provide more context about the nature of the error.
sess.execute(text("create extension if not exists vector;")) | ||
except Exception as e: | ||
if "already exists" not in str(e): | ||
raise ValueError( |
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.
As mentioned in the previous comment, consider raising a more specific error, such as a DatabaseError, instead of a ValueError.
Summary:
This PR improves the application's performance by increasing the number of uvicorn server workers and adds robust error handling to the database schema and extension creation process.
Key points:
--workers 8
to theCMD
instruction in theDockerfile
to increase the number of uvicorn server workers.vecs
schema andvector
extension creation process in the__init__
method of theClient
class inr2r/vecs/client.py
.Generated with ❤️ by ellipsis.dev