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

Implement KeySubstringAssetSelection #25800

Merged
merged 3 commits into from
Nov 14, 2024
Merged

Conversation

briantu
Copy link
Contributor

@briantu briantu commented Nov 8, 2024

Summary & Motivation

We want to support subset matching asset keys in the new asset selection syntax. Thus, we need to define a new KeySubstringAssetSelection class to support subset matching asset keys given a list of strings.

How I Tested These Changes

pytest python_modules/dagster/dagster_tests/asset_defs_tests/test_asset_selection.py::test_asset_selection_key_substring

@briantu
Copy link
Contributor Author

briantu commented Nov 8, 2024

Will add tests. Just wondering if I'm on the right track here. Some questions:

  • Do we want to make KeySubsetsAssetSelection a public API?
  • Also does the "key subset" sound a little confusing? Would "key substring" make more sense?

@briantu briantu marked this pull request as ready for review November 8, 2024 22:50
@briantu briantu changed the title Implement KeySubsetsAssetSelection Implement KeySubstringAssetSelection Nov 11, 2024
@briantu briantu force-pushed the briantu/subset-asset-selection branch 2 times, most recently from 79d6163 to 813f92b Compare November 12, 2024 16:37
@briantu briantu force-pushed the briantu/subset-asset-selection branch from 813f92b to 143fb12 Compare November 14, 2024 20:32
Copy link
Contributor Author

briantu commented Nov 14, 2024

Merge activity

  • Nov 14, 3:50 PM EST: A user started a stack merge that includes this pull request via Graphite.
  • Nov 14, 3:51 PM EST: Graphite rebased this pull request as part of a merge.
  • Nov 14, 3:53 PM EST: A user merged this pull request with Graphite.

@briantu briantu force-pushed the briantu/subset-asset-selection branch from 143fb12 to 0c00cca Compare November 14, 2024 20:51
@briantu briantu merged commit 31cae09 into master Nov 14, 2024
1 check failed
@briantu briantu deleted the briantu/subset-asset-selection branch November 14, 2024 20:53
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.

2 participants