dagster-io / dagster

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

DagsterInvalidDefinitionError for typing.Sequence #25490

Open hbruch opened 1 month ago

hbruch commented 1 month ago

What's the issue?

I'd like to at PEP 484 type annotations to my asset:

@asset
def my_asset(
    context: AssetExecutionContext, pipes_subprocess_client: PipesSubprocessClient
) -> Sequence['PipesExecutionResult']:

    return pipes_subprocess_client.run(
        command=['bash', 'my_script.sh'], context=context, cwd=SCRIPT_DIR
    ).get_results()

While this is linted without errors, this results in a DagsterInvalidDefinitionError:

The above exception was caused by the following exception:
dagster._core.errors.DagsterInvalidDefinitionError: Invalid type: dagster_type must be an instance of DagsterType or a Python type: got typing.Sequence[typing.Union[dagster._core.definitions.result.MaterializeResult, dagster._core.definitions.asset_check_result.AssetCheckResult]].

What did you expect to happen?

I don't expect a DagsterInvalidDefinitionError. Is typing.Sequence not (yet) supported?

How to reproduce?

No response

Dagster version

1.8.12

Deployment type

Local

Deployment details

No response

Additional information

No response

Message from the maintainers

Impacted by this issue? Give it a 👍! We factor engagement into prioritization. By submitting this issue, you agree to follow Dagster's Code of Conduct.

garethbrickman commented 1 month ago

I think ­this part of the Type docs covers cases like this:

if you wish to use a ​DagsterType​ alongside your annotations (to perform more complex validation than the default runtime typechecks), you should include this information in the relevant ​InputDefinition​ or ​OutputDefinition​.

There's an example of that ­here in the Examples section.

hbruch commented 3 weeks ago

Not sure if I misinterpret your answer, @garethbrickman. I'd expect sequence to be inferred from the type hint, as described in this section. Defining an ​InputDefinition​ or ​OutputDefinition​ would rather look as a workaround to me, as I don't want to perform additional checks on the returned result.