Page MenuHomeElementl

Enable default config values with direct solid invocation
ClosedPublic

Authored by cdecarolis on Wed, May 26, 11:23 PM.

Details

Summary

Default config values were broken on solid invocation. This fixes, and re-organizes some of the core flow. Now, we "bind" a DirectSolidExecutionContext into a "BoundSolidExecutionContext", which has its resources and config validated, and has the solid definition.

Issue tracking: https://github.com/dagster-io/dagster/issues/4216

Test Plan

Added additional unit tests for default config

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Add test for partial config binding

redfordnod

python_modules/dagster/dagster/core/execution/context/invocation.py
36–187

likely will want to go back and clean this up (different error emssages) given the bound thing - not in this diff tho

226–227

still seems odd to me that you have to manually handle defaulting here given process_config is supposed to do it.

Is this because we're getting config_field instead of config_schema?

253–255

could mention that it is bound to a specific solid that is about to be invoked

352–354

not critical for this diff: i think you can make this from the bare def - if we get pending invocationsa working this will be that

python_modules/dagster/dagster_tests/core_tests/test_solid_invocation.py
178–204

worth adding some more deeply nested and complex tests

This revision is now accepted and ready to land.Thu, May 27, 5:17 PM

Added more complex test cases, fixed docstring of BoundSolidExecutionContext, remove unnecessary if block