dagster-io / dagster

An orchestration platform for the development, production, and observation of data assets.
https://dagster.io
Apache License 2.0
11.15k stars 1.4k forks source link

Graph In/Out Mapping type alignment removal follow-up #7109

Open alangenfeld opened 2 years ago

alangenfeld commented 2 years ago

CompositeSolid enforced that types had to be aligned along input/output mapping. In the move to job/graph/op https://github.com/dagster-io/dagster/pull/4876 dropped dagster_type, effectively removing the enforcement.

Other parts of the system were potentially built making implicit assumptions about this type alignment which may require fixes or UX remediations.

Observed issues:

sryza commented 2 years ago

Why do all destinations for input mapping need to be the same type? I'm imagining a situation where the same outputs gets routed to inputs of two different ops. The output has 20 columns, but each of the ops only needs 5 columns to be present, and those 5 columns are different between the two ops.

alangenfeld commented 2 years ago

Why do all destinations for input mapping need to be the same type? ... output has 20 columns, but each of the ops only needs 5 columns to be present, and those 5 columns are different between the two ops

Thats a good point. Probably makes sense for the mappings to follow similar enforcement to our input/output type alignment rules aka basically nothing.

Will update the issue.

alangenfeld commented 2 years ago

GraphIn does not support dagster_type but you can set one via type hints I believe only allowing the information via typehints was unlikely an explicit design choice.

Some discussion on this from https://github.com/dagster-io/dagster/pull/7741, summarized: