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

SelectorConfig cannot handle intersections of config.tags tag selectors #694

Closed
adammarples opened this issue Nov 20, 2023 · 3 comments
Closed
Labels
area:config Related to configuration, like YAML files, environment variables, or executer configuration area:selector Related to selector, like DAG selector, DBT selector, etc parsing:custom Related to custom parsing, like custom DAG parsing, custom DBT parsing, etc priority:medium Medium priority issues are important issues that may have a workaround and medium impact

Comments

@adammarples
Copy link
Contributor

In order to select an intersection of tags while using LoadMode.CUSTOM I must use

select=["config.tags:mytag,config.tags:myothertag"]

Because selecting tags using LoadMode.CUSTOM is not working (#693)

When these tags are parsed in SelectorConfig.load_from_statement they are iterated over and only the last one is kept.

        items = statement.split(",") # ['config.tags:mytag', 'config.tags:myothertag']
        for item in items:
            ...
            elif item.startswith(CONFIG_SELECTOR):
                index = len(CONFIG_SELECTOR)
                key, value = item[index:].split(":") # key = tags
                if key in SUPPORTED_CONFIG:
                    self.config[key] = value # overwritten by last value in list "myothertag"
Copy link

dosubot bot commented Nov 20, 2023

🤖

Hello @adammarples,

Great to see you again and thanks for your continued contributions to the Cosmos project!

I'm diving into the issue you've raised regarding the SelectorConfig and intersections of config.tags tag selectors. I'll be back with a more comprehensive response shortly.

@dosubot dosubot bot added area:config Related to configuration, like YAML files, environment variables, or executer configuration area:selector Related to selector, like DAG selector, DBT selector, etc parsing:custom Related to custom parsing, like custom DAG parsing, custom DBT parsing, etc priority:medium Medium priority issues are important issues that may have a workaround and medium impact labels Nov 20, 2023
Copy link

dosubot bot commented Nov 20, 2023

🤖

Hello again @adammarples! Great to see you contributing to Cosmos repository once more.

Thank you for bringing this to our attention. It seems like you've identified a potential issue with the SelectorConfig class when handling multiple config tags.

In the current implementation, the load_from_statement function does indeed overwrite the value of a config key if it encounters the same key again in the statement. This is due to the line self.config[key] = value, which sets the value of the key in the config dictionary to the new value, effectively overwriting any previous value.

elif item.startswith(CONFIG_SELECTOR):
    index = len(CONFIG_SELECTOR)
    key, value = item[index:].split(":") # key = tags
    if key in SUPPORTED_CONFIG:
        self.config[key] = value # overwritten by last value in list "myothertag"

This behavior is different from how tags are handled, where multiple tags are appended to a list.

A potential solution could be to modify the load_from_statement function to append config values to a list, similar to how tags are handled. However, this would require a careful consideration of how to handle config keys that are expected to have a single value.

As for your question about similar issues, there was indeed a similar issue reported in the past: dbt/selectors doesn't work for config tags. This issue was resolved by a pull request: Fix selector for config tags. However, this fix was specifically for the dbt/selectors function and may not directly apply to your issue.

I hope this helps clarify the current behavior of the SelectorConfig class. If you have any further questions or need additional clarification, please let me know.

Sources


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.

Copy link

dosubot bot commented Feb 19, 2024

Hi, @adammarples,

I'm helping the Cosmos team manage their backlog and am marking this issue as stale. The issue you raised regarding the SelectorConfig's inability to handle intersections of config.tags tag selectors has been acknowledged and analyzed. There was a response providing potential insights into addressing the current problem, including a suggested solution involving modifying the load_from_statement function to append config values to a list, similar to how tags are handled.

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 it will be automatically closed in 7 days.

Thank you for your understanding and contribution to the Cosmos repository. If you have any further questions or updates, feel free to reach out.

I'm here to help,
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 19, 2024
@dosubot dosubot bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 26, 2024
@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 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:config Related to configuration, like YAML files, environment variables, or executer configuration area:selector Related to selector, like DAG selector, DBT selector, etc parsing:custom Related to custom parsing, like custom DAG parsing, custom DBT parsing, etc priority:medium Medium priority issues are important issues that may have a workaround and medium impact
Projects
None yet
Development

No branches or pull requests

1 participant