Skip to content

Deprecate example server #126

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

Merged
merged 1 commit into from
May 8, 2023

Conversation

Stonelinks
Copy link
Contributor

Split this in case these two implementations exist for a reason, but this depends on #125. Really its just the one commit at the end.

I found it confusing that two fastapi servers exist in this repo (more detail Stonelinks#2)

So this PR "deprecates" the example by pointing folks at the module one, but still remaining runnable.

@jmtatsch
Copy link

Found the second server confusing as well. And code duplication.

@Stonelinks Stonelinks force-pushed the deprecate-example-server branch 2 times, most recently from 6baee85 to eae21ac Compare May 1, 2023 17:43
@abetlen
Copy link
Owner

abetlen commented May 2, 2023

@Stonelinks I've updated the app server to use a more standard create_app pattern, can you update this example then I'm good to merge this in.

This commit "deprecates" the example fastapi server by remaining runnable but pointing folks at the module if they want to learn more.

Rationale:

Currently there exist two server implementations in this repo:

- `llama_cpp/server/__main__.py`, the module that's runnable by consumers of the library with `python3 -m llama_cpp.server`
- `examples/high_level_api/fastapi_server.py`, which is probably a copy-pasted example by folks hacking around

IMO this is confusing. As a new user of the library I see they've both been updated relatively recently but looking side-by-side there's a diff.

The one in the module seems better:
- supports logits_all
- supports use_mmap
- has experimental cache support (with some mutex thing going on)
- some stuff with streaming support was moved around more recently than fastapi_server.py
@Stonelinks Stonelinks force-pushed the deprecate-example-server branch from eae21ac to 0fcc25c Compare May 2, 2023 05:34
@Stonelinks
Copy link
Contributor Author

cool, yeah create_app is better than whatever i did!

fixed up the last commit on this branch

@abetlen abetlen merged commit 7499fc1 into abetlen:main May 8, 2023
xaptronic pushed a commit to xaptronic/llama-cpp-python that referenced this pull request Jun 13, 2023
* Update README.md

* Update README.md

remove facebook
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants