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.

Sign up for GitHub

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

Jump to bottom

[bitnami/mlflow] feat: Allow database dialects of external database to be configured #25965

Merged
merged 4 commits into from
Jun 4, 2024
Merged

[bitnami/mlflow] feat: Allow database dialects of external database to be configured #25965

merged 4 commits into from
Jun 4, 2024

Conversation

frittentheke
Copy link
Contributor

Description of the change

Currently the database URI statically uses postgresql as dialect, even though other types are supported by mlFlow.
This now allows to use the other supported database types as external database.

[1] https://mlflow.org/docs/latest/tracking/backend-stores.html?highlight=mssql#supported-store-types

Benefits

Other (mlFlow supported) database types can now be used as external databases

Possible drawbacks

Applicable issues

Additional information

Checklist

  • Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
  • All commits signed off and in agreement of Developer Certificate of Origin (DCO)

@github-actions github-actions bot added mlflow triage Triage is needed labels May 17, 2024
@github-actions github-actions bot requested a review from carrodher May 17, 2024 10:08
@frittentheke frittentheke mentioned this pull request May 18, 2024
Closed
@carrodher carrodher added verify Execute verification workflow for these changes in-progress labels May 18, 2024
@github-actions github-actions bot removed the triage Triage is needed label May 18, 2024
@github-actions github-actions bot removed the request for review from carrodher May 18, 2024 16:37
@github-actions github-actions bot requested a review from fmulero May 18, 2024 16:37
fmulero
fmulero reviewed May 20, 2024
View reviewed changes
Copy link
Collaborator

@fmulero fmulero left a comment

Choose a reason for hiding this comment

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

Thanks a lot @frittentheke for your contribution!

It looks great and very useful. I left a little suggestion, about the version used. Could you give it a glance?

bitnami/mlflow/Chart.yaml Outdated Show resolved Hide resolved
@frittentheke
[bitnami/mlflow] feat: Allow database dialect of external database to…
b6afc4b 
… be configured

Currently the database URI statically uses `postgresql` as dialect, even though
other types are supported by mlFlow.
This now allows to use the other supported database types as external database.

[1] https://mlflow.org/docs/latest/tracking/backend-stores.html?highlight=mssql#supported-store-types

Resolves: bitnami#25964
Signed-off-by: Christian Rohmann <christian.rohmann@inovex.de>
@bitnami-bot
Update CHANGELOG.md
045b475 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
@frittentheke
Copy link
Contributor Author

Thanks a lot @frittentheke for your contribution!

It looks great and very useful. I left a little suggestion, about the version used. Could you give it a glance?

DONE @fmulero . PTAL.

fmulero
fmulero previously approved these changes Jun 3, 2024
View reviewed changes
Copy link
Collaborator

@fmulero fmulero left a comment

Choose a reason for hiding this comment

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

Thanks a lot @frittentheke

LGTM

@fmulero fmulero enabled auto-merge (squash) June 3, 2024 16:33
@frittentheke
Copy link
Contributor Author

Thanks a lot @frittentheke

LGTM

Awesome @fmulero . Could you kindly check what is wrong in CI (https://github.com/bitnami/charts/actions/runs/9291472195/job/25745528664?pr=25965) keeping this from getting merged?

@carrodher
Merge branch 'main' into mlFlowDBDialects
058b78f 
Signed-off-by: Carlos Rodríguez Hernández <carlosrh@vmware.com>
carrodher
carrodher previously approved these changes Jun 4, 2024
View reviewed changes
@bitnami-bot
Update CHANGELOG.md
b20b436 
Signed-off-by: Bitnami Containers <bitnami-bot@vmware.com>
@fmulero fmulero disabled auto-merge June 4, 2024 10:35
@fmulero fmulero merged commit 6e2e370 into bitnami:main Jun 4, 2024
8 checks passed
@fmulero
Copy link
Collaborator

fmulero commented Jun 4, 2024

Thanks a lot @frittentheke
LGTM

Awesome @fmulero . Could you kindly check what is wrong in CI (https://github.com/bitnami/charts/actions/runs/9291472195/job/25745528664?pr=25965) keeping this from getting merged?

That was a temporary issue, not related with your changes

@frittentheke
Copy link
Contributor Author

Thanks @fmulero ! May I kindly ask you for an answer on how and when the container image will be updated?
See #22992 (comment)

I suppose #22992 is actually also resolved by this very pull request, provided the required libraries for MySQL + PostgreSQL have also been added.

@act-mreeves act-mreeves mentioned this pull request Jun 4, 2024
Open
37 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carrodher carrodher carrodher left review comments

@fmulero fmulero fmulero left review comments

Assignees

@fmulero fmulero

Labels
mlflow solved verify Execute verification workflow for these changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

MLFlow allow using other supported database backends as external database
4 participants
@frittentheke @fmulero @carrodher @bitnami-bot