feat: switch to upstream openwebui helm chart #379
Conversation
|
Thanks Kai! This is very helpful. Initially when we started there was no good openwebui helm chart, but now there is. We may eventually switch to no longer including openwebui by default but let's do that in a separate PR. Let me know if you need help with getting tests to pass again. |
|
Do we need to include this file in the repo itself? charts/kubeai/charts/open-webui-5.4.0.tgz I was assuming specify the dependency chart in Chart and chart repo url was enough. |
| version: 0.2.0 | ||
| - name: open-webui | ||
| condition: open-webui.enabled | ||
| repository: https://helm.openwebui.com/ |
There was a problem hiding this comment.
why do we need to include the tgz file itself if we already specify the repo here? I probably prefer to remove the tgz file and just point to upstream helm repo url.
There was a problem hiding this comment.
I have asked myself the same question. Does it still work without the tgz? I will try that.
There was a problem hiding this comment.
Without the tgz:
Error: INSTALLATION FAILED: An error occurred while checking for chart dependencies. You may need to run `helm dependency build` to fetch missing dependencies: found in Chart.yaml, but missing in charts/ directory: open-webui
There was a problem hiding this comment.
Will it automatically download the tgz file somehow and we can simply put it in .gitignore?
There was a problem hiding this comment.
I will play around with this a bit more as well to see if we can figure out a way to omit these tgz files. If not, we can just include them as well.
There was a problem hiding this comment.
Generated Artifacts:
The .tgz file created by helm dependency build is a generated artifact. Since it’s derived from the dependency information in your Chart.yaml, it can be recreated at any time by running that command.
Version Control Best Practices:
It’s generally best practice to avoid committing generated files to Git. Instead, you should commit the source (in this case, the dependency definitions in Chart.yaml) and let the build process handle generating the required artifacts.
Vendoring vs. Rebuilding:
The only time you might consider committing the .tgz files is if you need your chart to be completely self-contained (for example, to support offline installations or to ensure absolute reproducibility without fetching dependencies). However, if your users can run helm dependency build as part of their workflow, it’s simpler and cleaner not to commit them.
Publishing as a Helm Repo:
When you package your chart for publication in a Helm repo (using helm package), the packaging process will include your dependencies appropriately. The repository consumers will download the packaged chart, not the raw Git repo, so committing the .tgz file is unnecessary.
^^ AI had this to say. I think we're probably fine with not including the tgz as long as we run helm dependency build during local development. The publishing will automatically include the depedencies. I will make some changes to this PR to try this out.
There was a problem hiding this comment.
I was able to remove the tgz file from repo and it should be included in the release packaging workflow as well. I will make sure to validate that in the upcoming release.
kaiehrhardt
force-pushed
the
upstream-openwebui
branch
3 times, most recently
from
9988310 to
0257353
Compare
kaiehrhardt
force-pushed
the
upstream-openwebui
branch
from
0257353 to
bb1c4ae
Compare
| setValueTemplates: | ||
| open-webui.enabled: false | ||
| skipBuildDependencies: true |
There was a problem hiding this comment.
i'm not sure about that. do you want openwebui in your dev env?
There was a problem hiding this comment.
We can leave as-is for now. We probably should have 1 test that includes it at a minimum though.
Also updates the helm release process to include dependencies.
|
edit: fixed and added a commit I am seeing the following warning when testing locally: Will try to fix. |
``` helm install kubeai ./charts/kubeai W0205 21:53:19.307473 1133781 warnings.go:70] spec.template.spec.containers[0].env[4]: hides previous definition of "ENABLE_OLLAMA_API", which may be dropped when using apply ```
|
This new openwebui has some Arena Chat model that we should try to remove imo |
|
Confirmed it's all working well locally. Didn't know about the preview feature in Open WebUI
|
|
Thanks for your patience @kaiehrhardt I made a few follow up commits. Could you please review my changes before I merge it? The main things I changed are:
|
|
lgtm |
fixes #285