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

Tag selector not working with tests using the manifest json #580

Closed
nicohein opened this issue Oct 8, 2023 · 5 comments · Fixed by #615
Closed

Tag selector not working with tests using the manifest json #580

nicohein opened this issue Oct 8, 2023 · 5 comments · Fixed by #615
Labels
area:selector Related to selector, like DAG selector, DBT selector, etc bug Something isn't working

Comments

@nicohein
Copy link

nicohein commented Oct 8, 2023

There is a bug with the parsing of tests (at lease when using load_from_dbt_manifest).

Dbt does not populate the tags on the tests, still I would expected the behavior like in dbt that tests for the models matched by a tag are selected. The current implementation in astronomer-cosmos however only considers nodes in the filtered-notes for tests.

def load_from_dbt_manifest(self) -> None:

I would hope that this can be fixed by iterating through nodes not filtered_nodes in the following line:

for _, node in self.filtered_nodes.items():

Alternatively there could be a way to do a second run through the nodes in the selector to catch all tests that depend on selected models:

def select_nodes_ids_by_intersection(nodes: dict[str, DbtNode], config: SelectorConfig) -> set[str]:

I have not thought too much about possible side effects.

Current Workaround: Use a different selection method or explicitly tag tests.

--

astronomer-cosmos version 1.1.3
dbt-core vserion 1.5.7

@tatiana
Copy link
Collaborator

tatiana commented Oct 9, 2023

@nicohein This seems to be a limitation of Cosmos custom selector, which is used with LoadMethod.CUSTOM or LoadMethod.DBT_MANIFEST. If using load_via_dbt_ls, the selector behaviour would be just like dbt's.

Would you like to make a contribution to support this use-case in the custom selector?

@nicohein
Copy link
Author

@tatiana while I would like to make a contribution I am currently unable to give commitments. I am happy to be around for testing also beta versions within our test environment.

@tatiana tatiana added the bug Something isn't working label Oct 19, 2023
tatiana pushed a commit that referenced this issue Oct 30, 2023
…nd `LoadMethod.CUSTOM` (#615)

Resolving issues with the DBT_MANIFEST/CUSTOM load methods when the
has_test attribute is not assigned to the node correctly.

## Description

When a tag selector is used, all tests are filtered out because of the
DbtResourceType.TEST node does not have any information about tags. To
bypass this limitation - tags are assigned to tests based on their
parent model.

## Related Issue(s)

Closes: #580

Co-authored-by: edgarasnavickas <edgaras.navickas@kevin.eu>
tatiana pushed a commit that referenced this issue Nov 6, 2023
…nd `LoadMethod.CUSTOM` (#615)

Resolving issues with the DBT_MANIFEST/CUSTOM load methods when the
has_test attribute is not assigned to the node correctly.

## Description

When a tag selector is used, all tests are filtered out because of the
DbtResourceType.TEST node does not have any information about tags. To
bypass this limitation - tags are assigned to tests based on their
parent model.

## Related Issue(s)

Closes: #580

Co-authored-by: edgarasnavickas <edgaras.navickas@kevin.eu>
(cherry picked from commit 58de67e)
tatiana pushed a commit that referenced this issue Nov 6, 2023
…nd `LoadMethod.CUSTOM` (#615)

Resolving issues with the DBT_MANIFEST/CUSTOM load methods when the
has_test attribute is not assigned to the node correctly.

## Description

When a tag selector is used, all tests are filtered out because of the
DbtResourceType.TEST node does not have any information about tags. To
bypass this limitation - tags are assigned to tests based on their
parent model.

## Related Issue(s)

Closes: #580

Co-authored-by: edgarasnavickas <edgaras.navickas@kevin.eu>
(cherry picked from commit 58de67e)
@tatiana tatiana reopened this Nov 7, 2023
@tatiana tatiana added the area:selector Related to selector, like DAG selector, DBT selector, etc label Nov 8, 2023
Copy link

dosubot bot commented Feb 7, 2024

Hi, @nicohein,

I'm helping the Cosmos team manage their backlog and am marking this issue as stale. From what I understand, the issue "Tag selector not working properly with tests using manifest json in astronomer-cosmos" seems to have been resolved with a potential fix suggested in the code, along with a workaround provided. Additionally, there is an invitation for you to contribute to support this use-case, to which you expressed interest.

Could you please confirm if this issue is still relevant to the latest version of the Cosmos repository? If it is, please let the Cosmos team know by commenting on the issue. Otherwise, feel free to close the issue yourself, or the issue will be automatically closed in 7 days.

Thank you for your understanding and contributions to the Cosmos repository.

Dosu

@dosubot dosubot bot added the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Feb 7, 2024
@nicohein
Copy link
Author

nicohein commented Feb 7, 2024

HI @tatiana ,
I have written an alternative solution for dbt dags on airflow using offline parsing of the same. I will unfortunately not be able to contribute further.
Best
Nico

@dosubot dosubot bot removed the stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed label Feb 7, 2024
@dosubot dosubot bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 7, 2024
Copy link

dosubot bot commented Feb 7, 2024

🤖

Hi @nicohein,

Thank you for your response and for sharing your alternative solution. We appreciate your contributions to the Cosmos repository. We'll go ahead and close the issue now.

Best,
Dosu


This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:selector Related to selector, like DAG selector, DBT selector, etc bug Something isn't working
Projects
None yet
2 participants