example/llm: minor refactor

juspay / services-flake

NixOS-like services for Nix flakes

https://community.flake.parts/services-flake

MIT License

299 stars 28 forks source link

example/llm: minor refactor #227

Closed drupol closed 1 month ago

drupol commented 1 month ago

Hi!

I separated everything into small commits for easier review.

Let me know what you think!

shivaraj-bh commented 1 month ago

The purpose behind adding the model here was to ensure the user can try out inference on a model immediately after Open WebUI launches. If 4GB is too much, we can go with something smaller?

drupol commented 1 month ago

Actually, I removed it because open-webui is able to download it through the UI.

I believe that the goal of this example is to show users that open-webui works immediatelly, and they can configure ollama and the models to use through the web interface.

drupol commented 1 month ago

Please remove the formatting/ indentation changes as they are not necessary (nor in line with the rest of the project tree) and makes review more difficult.

That's the reason why I separated everything by commit. There's a standardised formatting now in Nix, are you sure you don't want it here?

open-webui is able to download it through the UI.

How straightforward a process is that? It looks like you have to go to settings -> Admin Panel -> Models and enter the model tag?

Yep, as simple as that indeed, very handy !

srid commented 1 month ago

example/llm: reformat with nixfmt-rfc-style

Actually I just saw this. @drupol I'd appreciate if you open different PRs for these. We use nixpkgs-fmt. What is nixfmt-rfc-style? Don't we have a CI check for formatting (I'd expect it to fail on this PR, but it doesn't @shivaraj-bh). Again, best to discuss this in its own PR.

drupol commented 1 month ago

OK !

srid commented 1 month ago

I'd say there should be three PRs,

Removing the model download (it being a non-trivial change)
Formatting change
Rest of the commits in here

srid commented 1 month ago

Rest of the commits in here

Regarding which:

No idea why this was removed.

drupol commented 1 month ago

Rest of the commits in here

Regarding which:

No idea why this was removed.

IMHO, I believe it's better to "show" the user of this project that data are saved within the same directory as the flake. He's free to change the dataDir of course.

To be honest that was the first thing that was kinda weird when I first tested this project. I expected the stateful data to be saved within the same directory and it was not the case.

srid commented 1 month ago

@drupol Would you like to join our Zulip? We are discussing a couple of your changes. I've responded to your last comment here as well:

https://nixos.zulipchat.com/#narrow/stream/414011-services-flake/topic/Require.20opinion.20on.20.60example.2Fllm.60/near/444705940