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

Extract helper function to find all subclasses of a marker interface #16793

Merged
merged 1 commit into from
Sep 27, 2023

Conversation

schrockn
Copy link
Member

@schrockn schrockn commented Sep 26, 2023

Summary & Motivation

This was a useful piece of logic implemented to do metaprogramming tests of our own code artifacts. I'd like to use it to do another test of this nature, so extracting into a helper function.

How I Tested These Changes

BK

@schrockn
Copy link
Member Author

schrockn commented Sep 26, 2023

@@ -328,3 +328,16 @@ def check_concurrency_claim(
if not self._sleep_interval:
return claim_status
return claim_status.with_sleep_interval(float(self._sleep_interval))


def get_all_direct_subclasses_of_marker(marker_interface_cls: Type) -> List[Type]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to name this to make it clear that it's looking at Dagster's top-level exports (or allow passing in some object that determines what scope to examine).

Copy link
Member Author

Choose a reason for hiding this comment

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

You have very accurately predicted a change I needed to make in the next PR: https://github.com/dagster-io/dagster/pull/16747/files#r1337902867 👍🏻

Will document it better there.

@schrockn schrockn merged commit 3c81fdd into master Sep 27, 2023
@schrockn schrockn deleted the reflection-helper branch September 27, 2023 02:04
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