Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: @observable_source_asset adapter to generate AssetsDefinition and SensorDefinition #16712

Conversation

schrockn
Copy link
Member

Summary & Motivation

This PR demonstrates how we could write an adapter to achieve backwards compatibility with existing observable_source_asset declarations.

In a test case we have the following code:

    defs = Definitions(
        assets=[create_assets_def_from_source_asset(an_asset)],
        sensors=[sensor_def_from_observable_source_asset(an_asset)],
    )

If we so chose, we could push down calling those functions against the source asset to within the Definitions __init__ method machinery to make this completely automatic. Whether or not that is a good idea is a different discussion.

How I Tested These Changes

BK

feedabck

Extract out observe_fn_to_op_compute_fn to it can be reused in other contexts

cp

feedback

experimental

cp
Comment on lines +409 to +433
@observable_source_asset
def an_asset() -> DataVersion:
return DataVersion("data_version")

# calling these helpers could be in the Definitions object itself
defs = Definitions(
assets=[create_assets_def_from_source_asset(an_asset)],
sensors=[sensor_def_from_observable_source_asset(an_asset)],
)

instance = DagsterInstance.ephemeral()

result = defs.get_implicit_global_asset_job_def().execute_in_process(instance=instance)
assert result.success

assert get_latest_asset_observation(instance, an_asset.key).data_version == "data_version"

sensor_def = defs.get_sensor_def(_get_auto_sensor_name(an_asset.key))

sensor_result = sensor_def.evaluate_tick(build_sensor_context(instance=instance))

assert len(sensor_result.asset_events) == 1
asset_observation = sensor_result.asset_events[0]
assert isinstance(asset_observation, AssetObservation)
assert asset_observation.asset_key == an_asset.key
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sryza this is the relevant test case to your interests

Copy link
Member Author

@schrockn schrockn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphite appears confused to respinning this here #16716 🤷‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant