-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[ENH] CLI Browse command #4431
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
[ENH] CLI Browse command #4431
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
7f3ad1a
to
b203660
Compare
412e817
to
cace3a2
Compare
documents: Option<Vec<Option<String>>>, | ||
uris: Option<Vec<Option<String>>>, | ||
metadatas: Option<Vec<Option<Metadata>>>, | ||
pub ids: Vec<String>, |
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.
Why are these made pub?
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 am constructing this payload for the CLI client's add requests. I can convert to using a new
function instead.
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.
Please don't expose these as pub. It deviates from other payloads. If you want to reuse this struct outside this crate please add a new() function that can package it for you.
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.
Sounds good. I'll issue a fix shortly
## Description of changes Adding the `chroma browse [collection name]` command for a TUI app to browse collections. Arguments: * `collection_name` (required) - the collection to browse * `--local` - find the collection in a local Chroma server. The CLI will try to connect to a Chroma server on localhost:8000, or stand up a local Chroma frontend if `chroma.config.yaml` is found in the current working directory. * `path` - data path for local Chroma. * `config` - path for a config file for local Chroma. * `host` - Chroma host with the collection. * `db` - the name of the DB with the collection. If "local" arguments are not provided, the CLI will assume this is a Cloud collection. If in this case `db` is not provided, the CLI prompts the user for selecting the DB with the collection. The code for the TUI app is under `rust/cli/tui/collection_browser`. The main files are: * `app_state.rs` - defines the state of the app. It also has functions that handle actions sent by the event handler, that should modify the state of the app. * `events.rs` - contains the event handling code in response to different key events, using a sender-receiver pattern. The `next` method uses `tokio::select` to handle both UI events and asynchronous operations (loading records, submitting queries). This pattern allows asynchronous operations to not block the UI. * `mod.rs` - contains the main app loop. On every iteration, we handle incoming events, fire asynchronous tasks, and draw the UI. ## Test plan - [ ] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Documentation Changes In a separate PR
Description of changes
Adding the
chroma browse [collection name]
command for a TUI app to browse collections.Arguments:
collection_name
(required) - the collection to browse--local
- find the collection in a local Chroma server. The CLI will try to connect to a Chroma server on localhost:8000, or stand up a local Chroma frontend ifchroma.config.yaml
is found in the current working directory.path
- data path for local Chroma.config
- path for a config file for local Chroma.host
- Chroma host with the collection.db
- the name of the DB with the collection.If "local" arguments are not provided, the CLI will assume this is a Cloud collection. If in this case
db
is not provided, the CLI prompts the user for selecting the DB with the collection.The code for the TUI app is under
rust/cli/tui/collection_browser
. The main files are:app_state.rs
- defines the state of the app. It also has functions that handle actions sent by the event handler, that should modify the state of the app.events.rs
- contains the event handling code in response to different key events, using a sender-receiver pattern. Thenext
method usestokio::select
to handle both UI events and asynchronous operations (loading records, submitting queries). This pattern allows asynchronous operations to not block the UI.mod.rs
- contains the main app loop. On every iteration, we handle incoming events, fire asynchronous tasks, and draw the UI.Test plan
pytest
for python,yarn test
for js,cargo test
for rustDocumentation Changes
In a separate PR