Skip to content

fix(dagster): stop the environment setting the knowledge graph's defaulted settings - #47

Merged
tillywoodfield merged 1 commit into
devfrom
fix/46-kg-settings-ignore-environment
Oct 27, 2026
Merged

tillywoodfield merged 1 commit into
devfrom
fix/46-kg-settings-ignore-environment

Conversation

@tillywoodfield

@tillywoodfield tillywoodfield commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

What & why

KnowledgeGraphResource.settings passed five fields to KGSettings and left LOCAL_PATH, GCP_CREDENTIALS, GCS_INPUT_FOLDER_PATH and GCS_OUTPUT_FOLDER_PATH to pydantic-settings, which reads them from the process environment and .env. Dagster loads the project's .env into the code location's environment, so a stray line there could move the ingestion's sync to a folder the workflow never reads, or apply a key to the upload only.

Closes #46

Changes

  • KnowledgeGraphResource.settings in src/dfo_pipelines/defs/resources.py passes the four fields explicitly. Constructor values take precedence over the environment and .env. gcs_input_folder_path takes its default from KGSettings.model_fields, so dcp-tools keeps ownership of it.
  • test_knowledge_graph_settings_ignore_environment in tests/test_resources.py sets all four in the environment, then in .env, and checks the settings match a clean-environment baseline.

Notes for reviewers

Checklist

  • Addresses one issue, linked above
  • Tests updated where relevant
  • Docs updated where relevant
  • Self-reviewed the diff

Testing: every step in .github/workflows/ci.yml ran locally and passed. Removing any one of the four fields fails both test cases.

🤖 Generated with Claude Code

@tillywoodfield

Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-27T13:33:52.666040Z 2722380 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: 27223806a4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@tillywoodfield
tillywoodfield marked this pull request as ready for review October 27, 2026 13:34
@tillywoodfield
tillywoodfield merged commit 5533af2 into dev Oct 27, 2026
4 checks passed
@tillywoodfield
tillywoodfield deleted the fix/46-kg-settings-ignore-environment branch October 27, 2026 13:37
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.

Stop the environment setting the KGSettings fields KnowledgeGraphResource leaves unset

1 participant