chore(ingestion): support dg for code location environment variables - #837
Merged
Conversation
The dagster-cloud CLI has no environment-variable command at all; dg does, but refuses to run outside a directory it recognises as a project. Adding the tool.dg blocks satisfies that check. It does not affect what the deployed code location loads, since dagster_cloud.yaml names the entry point explicitly -- verified by loading the definitions and running the suite with the blocks in place. The script sets each phase in one pass. Secrets go through --from-local-env rather than arguments, so they never reach the command line or shell history. Phases are separate deliberately. `database` waits on ingestion_role.sql having been run: pointing CLOUD_SQL_* at a role that does not exist makes database_connectivity fail in a way that looks like the serverless-to-Cloud-SQL problem it exists to test. Note for whoever runs this: it needs a Dagster+ *user* token. An agent token authenticates and returns data for queries, but is unauthorized for these mutations, and dg surfaces that as a KeyError on its own error handler rather than as a permission message. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Coverage✅ 79.26% total — gate is 75%. |
1 similar comment
Contributor
Coverage✅ 79.26% total — gate is 75%. |
CLOUD_SQL_USER means different things in the two authentication modes and db/engine.py passes it straight to the connector either way. The script set it to the plain role name unconditionally and then chose IAM auth when no password was exported, so the two settings contradicted each other -- a combination that fails as an authentication error looking like a missing grant. It now derives the value from whichever branch it takes. The role DDL leads with the IAM role for the same reason, since that is the configured path, and states the exact string CLOUD_SQL_USER has to match. IAM authentication also needs GCP-side grants that this configuration did not create: cloudsql.client, cloudsql.instanceUser, and the service account registered as a CLOUD_IAM_SERVICE_ACCOUNT database user. Without them the Postgres role exists but cannot be reached. They are gated on a cloud_sql_instance variable so the storage half can still be applied before the database half is decided. Variables are no longer set with --global. This Dagster+ deployment hosts other code locations, and deployment-level scope made the vendor and database credentials readable by all of them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
database_connectivity failed in Dagster+ with ModuleNotFoundError: No module named 'db', while the code location itself loaded and ingestion_heartbeat ran. The image copies the repository to /opt/dagster/app but never installs it: the generated requirements omit the project, and the build template only runs `pip install .` when a setup.py exists. So db and domain are importable only while that directory is on sys.path -- true when Dagster loads the code location, not guaranteed in the separate process that executes a step, which is exactly where the loader's lazy imports run. Locally an editable install puts the repository on sys.path unconditionally, which is why 43 tests pass and the failure appeared only once deployed. Two tests now cover it: one asserts the path entry exists, the other imports db from a process whose working directory is not the repository. Importing automated_ingestion now adds the repository root itself, so the coupling is satisfied wherever the package is imported from rather than depending on how it was launched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
Coverage✅ 79.27% total — gate is 75%. Coverage for the Python files changed in this PR
|
Contributor
|
Your pull request is automatically being deployed to Dagster Cloud.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Setting the code location's environment variables by hand in the UI is a dozen
clicks per variable across two scopes. This makes it three commands.
Why
dgand notdagster-cloudThe
dagster-cloudCLI has no environment-variable command at all — itsdeployment settingsandorganization settingscover deployment config, notenv vars. I grepped the installed package for secret/env-var mutations: nothing.
dg plus create envis the only CLI path.The
tool.dgblocksdgrefuses to run outside a directory it recognises as a project, which iswhat the earlier
MissingPyProjectDagsterBlockExceptionwas pointing at.I had deferred adding these because
dgcan auto-scandefs/**as componentsrather than importing a
Definitionsobject, and I did not want to change howthe location loads on a guess. Verified rather than assumed:
dagster_cloud.yamlnames the entry point explicitly, definitions still resolve all four assets with
the blocks in place, and the 41 tests pass.
The script
Three phases, each one pass:
Secrets use
--from-local-env, so values are read from the shell and neverappear as arguments — not in shell history, not in process listings.
storagesets a different bucket per scope:full(prod deployment) getsproduction,
branchgets staging.databaseis separate on purpose. It should wait untilautomated_ingestion/sql/ingestion_role.sqlhas been run. PointingCLOUD_SQL_*at a role that does not exist yet makes
database_connectivityfail in a waythat looks exactly like the serverless-to-Cloud-SQL problem the asset exists to
test. It also prefers IAM auth when no password is exported, which removes a
credential rather than rotating one.
Requires a user token
An agent token authenticates — a
currentDeploymentquery returns 200 —but is not authorized for these mutations.
dgreports that asKeyError: 'message'from its own error handler rather than as a permissionerror, which is a confusing hour if you do not know to check the token prefix.
Noted in the script header.
🤖 Generated with Claude Code