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

find_objects_in_module_of_types posible to load multiple times the same object when using variables. #27467

Open
axellpadilla opened this issue Jan 31, 2025 · 0 comments
Labels
type: bug Something isn't working

Comments

@axellpadilla
Copy link
Contributor

What's the issue?

I have a case where i just do this and this ends with duplicates but it was not posible to reproduce on an simple example:

all_assets = load_assets_from_current_module()

Assets are not really duplicated and this happens only when the .py is part of a bigger package with multiple relationships, and it was working ok on 1.9.6 (not on 1.9.7).

Looks like this function find_objects_in_module_of_types could generate duplicates reading the variable "all_assets" as a list on some cases, maybe cached results?

as an example doing:

all_assets = []
print(load_assets_from_current_module())

works ok.

Removing an import from a definitions.py also makes all_assets = load_assets_from_current_module() to work ok.

I suspect this part loads the same all_assets variable on the edge cases:

elif isinstance(value, list) and all(isinstance(el, types) for el in value):
            yield from value

because doing this all fixes the issue:

all_assets = tuple(load_assets_from_current_module())

What did you expect to happen?

Not loading the same variable that is using to save the load and avoiding duplicates or strange behaviors

How to reproduce?

I couldn't make a working test case and the original repo is private, but this should throw the error of duplicates (it doesn't, only on edge cases with import to a definition.py and even that isn't reproducible)

from dagster import asset, load_assets_from_current_module, AssetExecutionContext

@asset
def asset_1(context: AssetExecutionContext):
    X=1
    X+=1

@asset(deps=[asset_1])
def asset_2(context: AssetExecutionContext):
    X=2
    X+=1

all_assets = load_assets_from_current_module()

print(all_assets)

Dagster version

1.9.11

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.

@axellpadilla axellpadilla added the type: bug Something isn't working label Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant