-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
Helm deployment fails when postgresql.nameOverride is used #29214
Helm deployment fails when postgresql.nameOverride is used #29214
Conversation
Nit: Should we get a test for it to avoid regression? |
Makes sense to add unit tests @potiuk |
386bbfa
to
5c2c55f
Compare
…flow into issue-22790-nameOverride
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like static checks are failing. I'd also suggest that you set up the pre-commit hooks, they help catch these issues early 👍.
@jedcunningham @potiuk made the required changes. Can someone merge it once the CI passes? |
Just fixed all the test case issues. @potiuk / @jedcunningham can someone merge the PR once the CI passes? |
Helm property:
nameOverride
is not honoured properly for airflow postgresql charts and the connection string is not formed as expected. Fixes the improper url formation mentioned in: #22790.Testing results:
Without any override and release name being: adesai-airflow, namespace: airflow
postgresql://postgres:postgres@adesai-airflow-postgresql.airflow:5432/postgres?sslmode=disable
With override and same configs as above:
postgresql://postgres:postgres@overrideName:5432/postgres?sslmode=disable
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rst
or{issue_number}.significant.rst
, in newsfragments.