Page MenuHomeElementl

[crag] graph.with_resources
ClosedPublic

Authored by alangenfeld on Mon, May 17, 7:48 PM.

Details

Summary

A way to make an executable target from a graph

Test Plan

added test

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

python_modules/dagster/dagster/core/definitions/graph.py
341

went with the last name i saw used, most commonly referred to as bind though I think nobody loves that name

happy to rename this to whatever

python_modules/dagster/dagster/core/definitions/graph.py
341

worth taking in to consideration the name argument this will likely need to differentiate the same graph. I guess

name (Optional[str]): A name for this set of resources, only required to differentiate when the same graph is made executable multiple times with different resources.

isn't too bad.

I my head we will present legacy pipeline_name[mode_name] and graph_name[resources_name] the same way - square bracketed in this example

definitely not blocking on a first version, but would we plan to allow resource instances in addition to resource defs to open up the testing use case?

definitely not blocking on a first version, but would we plan to allow resource instances in addition to resource defs to open up the testing use case?

I think if we would make that change here we would make it across the board ie in modedef as well

python_modules/dagster/dagster/core/definitions/graph.py
341

config mapping also a dimension to consider - executable_with maybe?

python_modules/dagster/dagster/core/definitions/graph.py
341

I like with_resources a lot better.

For the mode_name disambiguation, we'll just have to be careful about the error messages. I think we'd currently get a 'non unique mode name' error?

python_modules/dagster/dagster/core/definitions/graph.py
341

No attachment to bind. I do think we'll end up needing some sort of generic name that spans both config-mapping and resources. Some other things we might end up wanting to allow people to supply at the time they're constructing an executable:

  • Executor (maybe this will go away)
  • Logger (maybe this will go away?)
  • Retry policy
  • Tags

I agree with @sandyryza 's generic name framing. At least in the short term, it's likely there's non-resource things that we will need to bind upwards. Even without considering executors and loggers, config definitely seems like something we would want to include here.

Mildly related: Should you be able to do this to a solid 🤔

Accepting to move this forward with the caveat of having a more generic name (for now)

This revision is now accepted and ready to land.Tue, May 25, 3:02 PM
This revision was automatically updated to reflect the committed changes.