Skip to content

Commit

Permalink
Fix upgrade story for new scheme field in postgres (#8220)
Browse files Browse the repository at this point in the history
Summary:
By changing the default to always include this, we made it so that old user code running against a new helm upgrade errors out. Instead, only set this in the instance config if they manually set it in the Helm chart.

Test Plan: BK
  • Loading branch information
gibsondan authored and johannkm committed Jun 9, 2022
1 parent cb800cc commit ed1b9b0
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from typing import Optional

from pydantic import BaseModel

from ...utils.kubernetes import ExternalImage
Expand All @@ -15,5 +17,5 @@ class PostgreSQL(BaseModel):
postgresqlPassword: str
postgresqlDatabase: str
postgresqlParams: dict
postgresqlScheme: str
postgresqlScheme: Optional[str]
service: Service
7 changes: 5 additions & 2 deletions helm/dagster/schema/schema_tests/test_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def helm_template() -> HelmTemplate:
)


@pytest.mark.parametrize("postgresql_scheme", ["postgresql", "postgresql+psycopg2"])
@pytest.mark.parametrize("postgresql_scheme", ["", "postgresql", "postgresql+psycopg2"])
@pytest.mark.parametrize("storage", ["schedule_storage", "run_storage", "event_log_storage"])
def test_storage_postgres_db_config(template: HelmTemplate, postgresql_scheme: str, storage: str):
postgresql_username = "username"
Expand Down Expand Up @@ -97,7 +97,10 @@ def test_storage_postgres_db_config(template: HelmTemplate, postgresql_scheme: s
assert postgres_db["db_name"] == postgresql_database
assert postgres_db["port"] == postgresql_port
assert postgres_db["params"] == postgresql_params
assert postgres_db["scheme"] == postgresql_scheme
if not postgresql_scheme:
assert "scheme" not in postgres_db
else:
assert postgres_db["scheme"] == postgresql_scheme


def test_k8s_run_launcher_config(template: HelmTemplate):
Expand Down
2 changes: 2 additions & 0 deletions helm/dagster/templates/helpers/instance/_postgresql.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,7 @@ postgres_db:
db_name: {{ .Values.postgresql.postgresqlDatabase }}
port: {{ .Values.postgresql.service.port }}
params: {{- .Values.postgresql.postgresqlParams | toYaml | nindent 4 }}
{{- if .Values.postgresql.postgresqlScheme }}
scheme: {{ .Values.postgresql.postgresqlScheme }}
{{- end }}
{{- end }}
1 change: 0 additions & 1 deletion helm/dagster/values.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion helm/dagster/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ postgresql:
# When set, overrides the default `postgresql` scheme for the connection string.
# (e.g., set to `postgresql+pg8000` to use the `pg8000` library).
# See: https://docs.sqlalchemy.org/en/13/dialects/postgresql.html#dialect-postgresql
postgresqlScheme: "postgresql"
postgresqlScheme: ""

service:
port: 5432
Expand Down

0 comments on commit ed1b9b0

Please sign in to comment.