Skip to content
This repository has been archived by the owner on Nov 13, 2024. It is now read-only.

CLI and server bug fixes on Windows #170

Merged
merged 7 commits into from
Nov 14, 2023

Conversation

igiloh-pinecone
Copy link
Contributor

Problem

Fixes multiple bugs when running the CLI on windows:

  1. On windows, calling CLI commands that use the server (e.g. canopy chat) didn't work.
  2. There was also a bug in inferring the /chat endpoint, which was using os.path.join()
  3. canopy stop tried to pgrep gunicorn, even though Windows doesn't have a pgrep command and Gunicorn isn't supported on windows.
  4. The /shutdown endpoint wasn't working correctly on Windows

Resolves #166.

Solution

  • The CLI used the url http://0.0.0.0:8000/ by default, which is not supported on windows. I replaced it with http://localhost:8000.
  • In windows we need to send the CTRL_C signal, which has a totally different implementation than SIGINT

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

Test Plan

Tested manually on mac, Windows and Linux.
We need to add a proper CLI unit test (known issue)

Two bugs:
1. Trying to `pgrep gunicorn` on Windows, even though windows doesn't have `pgrep` and Gunicorn isn't supported on windows.
2. In windows we need to send the `CTRL_C` signal, which has a totally different implmentation behind it than GITINT
Since there is no Gunicorn on windows, it doesn't make sense to tell the users to run it
The CLI used the url `http://0.0.0.0:8000` by default, which is not supported on windows.
I replaced it with `http://localhost:8000`.
@igiloh-pinecone igiloh-pinecone mentioned this pull request Nov 13, 2023
7 tasks
Copy link
Contributor

@miararoy miararoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small changes

src/canopy_cli/cli.py Outdated Show resolved Hide resolved
src/canopy_cli/cli.py Outdated Show resolved Hide resolved
@pinecone-io pinecone-io deleted a comment from relevance-bot Nov 14, 2023
Copy link
Contributor

@miararoy miararoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@igiloh-pinecone igiloh-pinecone added this pull request to the merge queue Nov 14, 2023
Merged via the queue into pinecone-io:main with commit 020a3de Nov 14, 2023
10 checks passed
@igiloh-pinecone igiloh-pinecone deleted the windows_fixes branch November 14, 2023 10:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI Chat wont start
2 participants