[RFC] tests to specify APIs for constructing jobs with check selections #16185
Conversation
I agree we need to keep |
100% agree with this call 👍🏻 |
|
I think this is ready for wider consumption. I've heard mixed feedback about our entire selection regime, so folks might have an opinions on changing this API without doing other changes. You only get so many shots as introducing a class with an central-sounding name like |
@schrockn I pushed an update that uses
I think we should get here eventually, but I would lean against targeting this change for launch week / 1.5 release. To do so, we'd need to either (a) mark I think (a) is risky and we don't have the bandwidth to mitigate that risk. I think (b) is pretty undesirable. |
Good point - created a discussion: https://github.com/dagster-io/internal/discussions/6621 |
Implementation of #16185, with some modifications - Use AssetSelection, rather than introducing AssetCheckSelection - externally use Tuple[AssetKey, str] rather than making AssetCheckHandle public - internally use the existing `resolve` method for assets, and `resolve_checks` for checks This means that most of the existing callsites that only care about selected assets (e.g. https://github.com/dagster-io/dagster/blob/johann/09-18-checks_on_AssetSelection/python_modules/dagster/dagster/_core/definitions/asset_graph.py#L118, https://github.com/dagster-io/dagster/blob/johann/09-18-checks_on_AssetSelection/python_modules/dagster/dagster/_core/definitions/data_time.py#L164) can keep doing what they're doing, while the callsites that care can get the checks. We'll expand the number of callsites to `resolve_checks` in the future- e.g. for sensors that return run requests with lists of asset keys. Question: Should we default to including or excluding checks? E.g. in `AssetSelection.all()` and `AssetSelection.assets(...)` - we've gone with including
|
implemented in #16610 |
Summary & Motivation
This is a set of unit tests that aim to specify behavior for how you construct jobs where you want to exercise fine-grained control over checks that execute in them.
Examples of fine-grained control:
Some things to point out here:
AssetSelectionreplaced withSelection. But I think we should keepAssetSelectionworking at least for backcompat.define_asset_jobandUnresolvedAssetJobDefinitionrather than introducing a new API. The idea is that a job that executes only asset checks is still an "asset job", because it operates on assets.How I Tested These Changes
These changes are tests.