Skip to content
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

fix(mlflow): Remove redundant Mlflow extra envs mapping #430

Conversation

deadlycoconuts
Copy link
Contributor

@deadlycoconuts deadlycoconuts commented Jan 8, 2025

Motivation

In PR #426, modifications were made to the Mlflow deployment template in order to allow it to be configured with additional environment variables specified under the extraEnvs field.

However that was problematic as there was already an existing template block that already parses the values of extraEnvs as environment values in the deployment template.

See the following lines:

This PR cleans up the previous template block for the extraEnvs field in favour of the new one, which is simpler.

Modification

  • charts/merlin/templates/mlflow-deployment.yaml - removal of a redundant block to parse environment variables

Checklist

  • Chart version bumped
  • README.md updated

@deadlycoconuts deadlycoconuts added the bug Something isn't working label Jan 8, 2025
@deadlycoconuts deadlycoconuts self-assigned this Jan 8, 2025
@deadlycoconuts deadlycoconuts marked this pull request as ready for review January 8, 2025 09:00
@deadlycoconuts deadlycoconuts requested a review from a team as a code owner January 8, 2025 09:00
@deadlycoconuts
Copy link
Contributor Author

Thanks for the quick review! Merging this now! 🚀

@deadlycoconuts deadlycoconuts merged commit 5a27031 into caraml-dev:main Jan 8, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants