Skip to content

Add examples directory to Mlflow chart #57

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

diamonwiggins
Copy link
Member

No description provided.

Copy link
Contributor

@scottrigby scottrigby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is great! We can always add more/refine later too. Some suggestions and a question for ya as well

nginx.ingress.kubernetes.io/ssl-redirect: "false"
tls:
enabled: true
genSelfSignedCert: true =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo fix

Suggested change
genSelfSignedCert: true =
genSelfSignedCert: true

protocol: https
host: "s3.amazonaws.com"
port: 443
ignoreTls: false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs newline at EOF

Suggested change
ignoreTls: false
ignoreTls: false

mlflow:
backendStore:
# Use an existing secret containing the database connection string
existingSecret: "mlflow-postgres-secret"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs newline at EOF

Suggested change
existingSecret: "mlflow-postgres-secret"
existingSecret: "mlflow-postgres-secret"

-f ./charts/mlflow/examples/database/embedded/values.yaml \
-f ./charts/mlflow/examples/network/ingress/values.yaml
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diamonwiggins I know we discussed on Slack the idea of adding comments about sensitive values to the main values.yaml, and I agree it's not the best place for it.

But since we're informing folks how to do this by example here, what do you think about taking the opportunity to ensure they follow our example securely? What do you think about something like this?

Suggested change
Be sure that files containing unencrypted sensitive values are never committed to Git. To address this, you can use tools like Sealed Secrets or SOPS to encrypt the values in the file. Or avoid adding these to values files altogether by using `--set` on the command line, which can be combined with the above techniques:
```bash
helm install mlflow ./charts/mlflow \
-f examples/database/external/direct-credentials.yaml \
--set postgres.auth.username=foo \
--set postgres.auth.password=bar

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.

2 participants