Page MenuHomeElementl

Remove checksum annotations in Helm user code deployments
ClosedPublic

Authored by rexledesma on Mar 10 2021, 9:21 PM.

Details

Summary

Updates https://dagster.phacility.com/D5608.

Dagit should be redeployed if the workspace.yaml changes,
but user code containers should not need to redeploy if the Dagit instance changes.

Saw this coupling of Dagit and User code deployments in the Helm chart while splitting the latter into the subchart.

Test Plan

bk
integration

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

rexledesma edited the summary of this revision. (Show Details)
johann requested changes to this revision.Mar 11 2021, 3:35 PM

user code containers should not need to redeploy if the Dagit instance changes.

Why not? I think you're right that workspace.yaml changes may not be necessary, but the userDeployments load dagster.yaml. Things like sensor code get the instance in their context

This revision now requires changes to proceed.Mar 11 2021, 3:35 PM

@johann the userDeployments load the instance ref, but it originates from the request (i.e. Dagit or Daemon). If Dagit or the Daemon have restarted, then on subsequent requests to fetch sensors or schedules, the updated instance should be passed in, regardless of whether the userDeployments have been updated or not - looking at https://sourcegraph.com/github.com/dagster-io/dagster/-/blob/python_modules/dagster/dagster/grpc/impl.py#L237:1 and https://sourcegraph.com/github.com/dagster-io/dagster/-/blob/python_modules/dagster/dagster/grpc/impl.py#L195:1

nice find! doesnt seem like user deployment loads the instance configmap, so looks safe to me

Macro pooh_eating_honey:

This revision was not accepted when it landed; it landed in state Needs Review.Mar 18 2021, 7:59 AM
This revision was automatically updated to reflect the committed changes.