-
Notifications
You must be signed in to change notification settings - Fork 121
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.
@miararoy see a few suggestions
src/canopy_cli/cli.py
Outdated
@click.option("--host", default="localhost", | ||
help="Hostname or address to bind the server to. Defaults to localhost") |
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 particular change wasn't tested on all OSes (passing host='localhost'
to uvicorn.run()
).
If you wanna merge it, we will need to test it.
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.
does 0.0.0.0 works?
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.
on all OSs - that is
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.
If that was the code in main
before this PR - then yeah. I tested this code on all 3 OSes manually yesterday.
@@ -4,6 +4,8 @@ | |||
|
|||
from canopy.models.data_models import Messages, Query, Document | |||
|
|||
# TODO: consider separating these into modules: Chat, Context, Application, etc. |
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.
👍
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.
@miararoy LGTM, but see this comment before merging
Problem
Versioning an API is a crucial aspect of maintaining its usability and compatibility as it evolves over time. There are several approaches to API versioning but the most common is URI Versioning (Path Versioning): where the version number is included in the URL path. For example:
Solution
Type of Change
Test Plan
Regression