Add http endpoints for sidecar functionality by gjreda · Pull Request #359 · refstudio/refstudio

gjreda · 2023-08-16T20:50:01Z

part of #347

Running this PR

cd refstudio/python

poetry run uvicorn web:api --reload

You should then be able to visit http://127.0.0.1:8000/api/v0/docs in your browser and test various endpoints. Not all of them have been implemented yet, and some need some slight changes due to the way the current sidecar works (i.e. since we communicate via stdout, maybe of the existing sidecar functions do not return anything ... which needs to happen for the API).

Example usage

curl -s -X 'POST' \
  'http://127.0.0.1:8000/api/sidecar/ai/rewrite' \
  -H 'accept: application/json' \
  -H 'Content-Type: application/json' \
  -d '{
  "text": "Chicago is the most populous city in the U.S. state of Illinois and the third-most populous in the United States after New York City and Los Angeles. With a population of 2,746,388 in the 2020 census, it is also the most populous city in the Midwest. As the seat of Cook County (the second-most populous U.S. county), the city is the center of the Chicago metropolitan area, one of the largest in the world.",
  "manner": "concise",
  "n_choices": 1,
  "temperature": 0.7
}' | jq

{
  "status": "ok",
  "message": "",
  "choices": [
    {
      "index": 0,
      "text": "Chicago is the most populous city in Illinois and the third-most populous in the United States, after New York City and Los Angeles. It has a population of 2,746,388 according to the 2020 census, making it the most populous city in the Midwest. It is also the center of the Chicago metropolitan area, which is one of the largest in the world and is located in Cook County, the second-most populous county in the US."
    }
  ]
}

codecov · 2023-08-16T20:53:27Z

Codecov Report

Merging #359 (a216d67) into main (c1bebd9) will increase coverage by 0.33%.
Report is 2 commits behind head on main.
The diff coverage is 88.75%.

@@            Coverage Diff             @@
##             main     #359      +/-   ##
==========================================
+ Coverage   84.42%   84.75%   +0.33%     
==========================================
  Files         157      169      +12     
  Lines        9372     9684     +312     
  Branches     1022     1056      +34     
==========================================
+ Hits         7912     8208     +296     
- Misses       1449     1465      +16     
  Partials       11       11

Files Changed	Coverage Δ
python/main.py	`0.00% <0.00%> (ø)`
python/sidecar/ingest.py	`89.70% <72.72%> (-0.67%)`	⬇️
python/sidecar/http.py	`88.63% <88.63%> (ø)`
python/sidecar/chat.py	`93.65% <100.00%> (+0.10%)`	⬆️
python/sidecar/rewrite.py	`94.20% <100.00%> (+0.17%)`	⬆️
python/sidecar/search.py	`51.35% <100.00%> (+2.77%)`	⬆️
python/sidecar/storage.py	`97.70% <100.00%> (+7.33%)`	⬆️
python/web.py	`100.00% <100.00%> (ø)`

... and 21 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

danvk

Looks good! Two high-level bits of feedback:

It would be nice to have some tests
Does this do validation of request bodies for us?

We'll want to get these endpoints into our codegen setup but that can be in a follow-up PR.

Also I'm seeing this error when I try to use the GET /ingest/references endpoint:

TypeError: Failed to execute 'fetch' on 'Window': Request with GET/HEAD method cannot have body.

cguedes

Agree with Dan's comments.

We should have a distinct entry point for the HTTP server (from the cli/sidecar).

I see the backend for HTTP to translate http parameters (body, querystring) and call the internal functions that return a value. Then, that value is sent back to the client serialized as JSON.

For the CLI application interface, I see the app receiving requests in JSON (...Request) and also call the internal function that return a value. Then is the responsibility of the CLI to translate that to a sys.stdout.write(response.json()) reply.

The HTTP endpoints should also be adjusted to:

use the first segment to scope the different backend scopes:
- references (ingestion, status, edit, delete, ...),
- ai (completion, chat)
- search (s2)
adopt the best HTTP method
- GET to access a resource or query information
- I think we should use this method for text completion and for s2 search)
- DELETE to delete a resource (no need to repeat delete in the URL, we should have an URL that identify a resource)
- PATCH to partially update a reference

Alternatively, if we want to mimic the sidecar request/reply method of sending a JSON payload and receiving a JSON payload, we should only use POST to a api/v0/sidecar/... endpoint.

We also need to discuss how the settings (ex: OPENAI_API_KEY) are sent/accessible in the HTTP server. We know that the setting is configured by the client.

gjreda · 2023-08-17T15:26:14Z

For the CLI application interface, I see the app receiving requests in JSON (...Request) and also call the internal function that return a value. Then is the responsibility of the CLI to translate that to a sys.stdout.write(response.json()) reply.

@cguedes I agree with this approach, though it's a fairly sizable refactor (mainly the tests) that I think would be distracting in this PR. I'll add it as a follow up.

danvk · 2023-08-17T20:10:18Z

 import json

-from sidecar import chat, cli, ingest, rewrite, storage, search
+from sidecar import chat, cli, ingest, rewrite, search, storage


Not necessary for this PR, but is there a linter/formatter that's not running as part of CI?

Honestly, this is something my vs-code only does for refstudio and I haven't been able to figure out why

I had thought it might have been due to prettier integration, but seems not

Add http endpoints for sidecar functionality

e50758b

Add some returns

9ffe857

danvk previously approved these changes Aug 16, 2023

View reviewed changes

Comment thread python/sidecar/http.py Outdated

Comment thread python/sidecar/http.py

Comment thread python/sidecar/http.py Outdated

Comment thread python/main.py Outdated

Comment thread python/sidecar/http.py

cguedes requested changes Aug 17, 2023

View reviewed changes

gjreda added 2 commits August 17, 2023 10:39

Address PR comments

9886c08

Add tests

1d6622c

gjreda dismissed danvk’s stale review via 1d6622c August 17, 2023 19:55

gjreda requested review from cguedes and danvk August 17, 2023 20:06

fix tests

9729f28

danvk previously approved these changes Aug 17, 2023

View reviewed changes

gjreda dismissed danvk’s stale review via 9729f28 August 17, 2023 20:15

gjreda marked this pull request as ready for review August 17, 2023 20:18

danvk previously approved these changes Aug 17, 2023

View reviewed changes

rename routes

a216d67

gjreda dismissed danvk’s stale review via a216d67 August 17, 2023 20:27

cguedes approved these changes Aug 17, 2023

View reviewed changes

cguedes merged commit eb78d8c into main Aug 17, 2023

cguedes deleted the http-endpoints-fastapi branch August 17, 2023 20:36

danvk mentioned this pull request Aug 17, 2023

Allow Ref Studio to run locally as a web app #347

Closed

gjreda mentioned this pull request Aug 17, 2023

Separate CLI interprocess comms from functionality #367

Closed

cguedes mentioned this pull request Aug 18, 2023

Frontend sidecar.ts facade that calls Tauri or HTTP backend APIs #380

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add http endpoints for sidecar functionality#359

Add http endpoints for sidecar functionality#359
cguedes merged 6 commits into
mainfrom
http-endpoints-fastapi

gjreda commented Aug 16, 2023 •

edited

Loading

Uh oh!

codecov Bot commented Aug 16, 2023 •

edited

Loading

Uh oh!

danvk left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cguedes left a comment •

edited

Loading

Uh oh!

gjreda commented Aug 17, 2023

Uh oh!

danvk Aug 17, 2023

Uh oh!

gjreda Aug 17, 2023

Uh oh!

gjreda Aug 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

gjreda commented Aug 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Running this PR

Example usage

Uh oh!

codecov Bot commented Aug 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

danvk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cguedes left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gjreda commented Aug 17, 2023

Uh oh!

danvk Aug 17, 2023

Choose a reason for hiding this comment

Uh oh!

gjreda Aug 17, 2023

Choose a reason for hiding this comment

Uh oh!

gjreda Aug 17, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gjreda commented Aug 16, 2023 •

edited

Loading

codecov Bot commented Aug 16, 2023 •

edited

Loading

cguedes left a comment •

edited

Loading