Blueprint and load_defs_from_yaml

[sryza]

Summary & Motivation

How I Tested These Changes

[benpankow]

I think it makes sense that this is Type[Blueprint] at least right now. In the future it might be ergonomic to allow for for e.g. List[Blueprint] (which would just have a special cased build_defs impl, I guess).

[benpankow]

I am a little curious at the decision to specify the blueprint type in Python code - how would we suggest users implement some sort of "union type" where a file could define one of a few types of assets?

I could imagine a few ways to do this:

• Separate load_defs_from_yaml calls targeting each set of files. This means users either would (i) separate their blueprint types into subfolders, (ii) distinguish them with something in the filename or (iii) have to introspect the files to figure out the type
• A wrapper Blueprint type with a Selector/union type at the top level. This means you need to have some type which imports/explicitly unions all possible blueprints (pydantic & our type system's handling of unions is a bit wonky)

I think it's a reasonable API decision but I do find the other option - letting the user encode the blueprint type in the YAML file - also pretty nice:

_blueprint:
  module: my_module.blueprints
  class: MyFooBlueprint

a_str: foo
an_int: 55

This is less elegant, but avoids user code having to know about all possible blueprints, or defining a big wrapper type. This would be useful for things like integration-provided blueprints:

_blueprint:
  module: dagster_embedded_elt.sling
  class: SlingSyncBlueprint

...

_blueprint:
  module: dagster_dbt
  class: DbtModelBlueprint

...

That way a user can just call

defs = load_defs_from_yaml(...)

With a dir full of various YAML files that point at different integrations, as opposed to multiple calls which have to filter out each match/non-match.

[benpankow]

There's also a world in which specifying blueprint-in-yaml could nicely match syntactically with linking a YAML schema for editor type checking:

# dagster: blueprint=dagster_embedded_elt.sling.SlingSyncBlueprint
# yaml-language-server: $schema=./.schemas/dagster_embedded_elt.sling.SlingSyncBlueprint

and we'd have some utility dagster blueprint generate-schemas to populate that .schemas folder automatically

[sryza]

avoids user code having to know about all possible blueprints

The way I have been thinking about it, this is a feature more than a bug. I.e. a big goal of blueprints is to help platform owners create guardrails for their stakeholders. By defining the set of available blueprints up front, you can be confident that only those will show up in the loaded definitions.

[sryza]

I think it makes sense that this is Type[Blueprint] at least right now. In the future it might be ergonomic to allow for for e.g. List[Blueprint] (which would just have a special cased build_defs impl, I guess).

Agreed. I actually started implementing this in this PR, but it added enough complexity that I figured it would be better to consider it separately. Same with unions.

[benpankow]

naming q, impl is good 🚢

[benpankow]

Definitions feels like a first-class noun in the Dagster system and at first glance I would expect BlueprintDefinitions to implement it. There's an unfortunate collision with the lowercase d "definitions", which this seems to actually be describing, but this threw me for a bit of a loop.

How would you feel about e.g. BlueprintDefs or even PartialDefinitions, DefinitionsFragment (leaving open the door for non-Blueprint use-case)?

Is there a world in which we'd have some sort of definitions ABC that would let you .merge a BlueprintDefinitions with Definitions?

[benpankow]

Is BlueprintDefinitions replacing the concept of merging/joining Defintions? (eg are we returning to the world in which Definitions is singular)?

[sryza]

Chatted this through in person, but for posterity:

• I hope to eliminate BlueprintDefinitions and replace it with Definitions
• Using Definitions here will require making it possible to construct a Definitions object without binding all the resource keys to resource definitions, which is a behavior change from the current Definitions object. If we do make that change, we will probably want to at least wait until 1.8. Adding a new class allows us to make progress on blueprints without waiting. More discussion here: Definitions.merge and don't immediately validate definitions #21746.

[sryza]

@benpankow actually I updated this to handle unions. It just required switching from cls.parse_obj(...) to parse_obj_as(cls, ...). I also added a couple tests.