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

Refresh Metaflow charts and create a github pages to host them #60

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

josephsirak
Copy link
Contributor

No description provided.

@josephsirak josephsirak changed the title Create a github pages to host metaflow helm charts Refresh Metaflow charts and create a github pages to host them Dec 16, 2024
charts/metaflow/charts/metaflow-service/Chart.yaml Outdated Show resolved Hide resolved

image:
pullPolicy: IfNotPresent
repository: public.ecr.aws/outerbounds/metaflow_metadata_service
Copy link
Contributor

Choose a reason for hiding this comment

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

is there some automation regarding the release of this image? OSS only releases under netflixoss/metaflow_metadata_service

Copy link
Contributor Author

@josephsirak josephsirak Jan 3, 2025

Choose a reason for hiding this comment

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

Yes, we have a pipeline that listens to the repository and publish builds to ecr

charts/metaflow/charts/metaflow-ui/Chart.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@saikonen saikonen left a comment

Choose a reason for hiding this comment

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

#50 might still be a relevant change for this as well.

Copy link
Contributor

@saikonen saikonen left a comment

Choose a reason for hiding this comment

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

also one big stumbling block for people deploying on RDS with a new (v15+ IIRC) postgres engine is that the metadata/ui-backend versions default to no ssl connection for the db, but later versions of postgres have started to mandate this.

It would be a good idea to include this env var in the templates for the services, to ensure db connectivity in case users start off with the provided postgres, but later on switch to RDS:

MF_METADATA_DB_SSL_MODE=prefer

see #58 for similar fixes regarding cloudformation

@josephsirak
Copy link
Contributor Author

#50 might still be a relevant change for this as well.

Added as separate commit 9566c9d

@josephsirak
Copy link
Contributor Author

also one big stumbling block for people deploying on RDS with a new (v15+ IIRC) postgres engine is that the metadata/ui-backend versions default to no ssl connection for the db, but later versions of postgres have started to mandate this.

It would be a good idea to include this env var in the templates for the services, to ensure db connectivity in case users start off with the provided postgres, but later on switch to RDS:

MF_METADATA_DB_SSL_MODE=prefer

see #58 for similar fixes regarding cloudformation

Added a default in values.yaml: 80cd398

@josephsirak josephsirak requested a review from saikonen January 7, 2025 18:19
Copy link
Contributor

@saikonen saikonen left a comment

Choose a reason for hiding this comment

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

approved pending testing results. the charts seem to apply correctly on a minikube cluster at least :)

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