fix: stuff #312
fix: stuff #312
Conversation
feat: Mcpo
|
Hello. It does not seem to comply with CONTRIBUTING.md at all. Please consider about it. |
westbrook-ai
left a comment
westbrook-ai
left a comment
There was a problem hiding this comment.
I appreciate the efforts on this PR, but there is a lot more work needed:
- PR name is not helpful, please make it accurately reflect what this PR is trying to add (a new MCPO subchart)
- Please ensure that required Github Actions workflow updates are included - we should have a new MCPO test workflow, see example here: https://github.com/open-webui/helm-charts/blob/main/.github/workflows/helm-test-pipelines.yml
- Please address other concerns in my review comments
Once these are addressed, I will do more detailed testing on this. Thank you again!
| parent: pipelines.service | ||
| condition: pipelines.enabled | ||
| - name: mcpo | ||
| repository: file://../mcpo |
There was a problem hiding this comment.
I would prefer that we use the same approach as what we use for the Pipelines chart, which you can see right above this entry.
| # incremented each time you make changes to the application. Versions are not expected to | ||
| # follow Semantic Versioning. They should reflect the version the application is using. | ||
| # It is recommended to use it with quotes. | ||
| appVersion: "main" |
There was a problem hiding this comment.
main is not an appropriate appVersion, considering the latest release of Open WebUI MCPO is v0.0.19. While I know this is not currently available as a tag on the Docker images, updated the values.yaml entry to use the main tag as the default will address this. I will speak to the maintainer about getting version tags on the MCPO images, but I think this is the best solution for now.
3 participants
No description provided.