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

Raise an exception if ProfileConfig is instantiated with both profile_mapping and profiles_yml_filepath #656

Closed
tatiana opened this issue Nov 8, 2023 · 1 comment · Fixed by #681
Labels
area:profile Related to ProfileConfig, like Athena, BigQuery, Clickhouse, Spark, Trino, etc bug Something isn't working good first issue Good for newcomers
Milestone

Comments

@tatiana
Copy link
Collaborator

tatiana commented Nov 8, 2023

As of Cosmos 1.2.2, it does not complain if users set both profile_mapping and profiles_yml_filepath in a ProfileConfig instance. Cosmos should raise an error in this scenario.

This issue was noticed in the Slack thread:
https://apache-airflow.slack.com/archives/C059CC42E9W/p1699365889141509

@tatiana tatiana added bug Something isn't working good first issue Good for newcomers labels Nov 8, 2023
@tatiana tatiana added this to the 1.3.0 milestone Nov 8, 2023
Copy link

dosubot bot commented Nov 8, 2023

🤖

Hello @tatiana,

Nice to meet you! I'm Dosu, a bot here to assist you with the Cosmos repository. I'm here to help troubleshoot bugs, answer questions, and guide you on your journey as a contributor while we wait for a human maintainer. Let's work together on this!

Thank you for bringing this to our attention. You're correct that the current behavior of the ProfileConfig class in Cosmos v1.2.2 does not raise an exception if both profile_mapping and profiles_yml_filepath are set. Instead, it prioritizes the user-supplied profiles.yml file specified by profiles_yml_filepath and only uses profile_mapping if profiles_yml_filepath is not set or does not exist.

To change this behavior and raise an exception if both profile_mapping and profiles_yml_filepath are set, you can add a validation in the __post_init__ method of the ProfileConfig class. Here's an example of how you can do this:

def __post_init__(self) -> None:
    "Validates that we have enough information to render a profile."
    # if using a user-supplied profiles.yml, validate that it exists
    if self.profiles_yml_filepath and not Path(self.profiles_yml_filepath).exists():
        raise CosmosValueError(f"The file {self.profiles_yml_filepath} does not exist.")
    
    # raise an exception if both profiles_yml_filepath and profile_mapping are set
    if self.profiles_yml_filepath and self.profile_mapping:
        raise CosmosValueError("Both profiles_yml_filepath and profile_mapping cannot be set at the same time.")

This code will raise a CosmosValueError if both profiles_yml_filepath and profile_mapping are set when a ProfileConfig instance is created. The CosmosValueError is a custom exception class in the Cosmos codebase that is used to handle invalid configuration values.

Please note that this change will need to be tested thoroughly to ensure it doesn't introduce any unexpected behavior or break any existing functionality. If you need further assistance, feel free to ask.

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.

@tatiana tatiana added the area:profile Related to ProfileConfig, like Athena, BigQuery, Clickhouse, Spark, Trino, etc label Nov 8, 2023
jbandoro pushed a commit to jbandoro/astronomer-cosmos that referenced this issue Nov 20, 2023
…ERNETES` (astronomer#681)

This PR moves validation of the `profiles_yml_path` to later in the dag
generation process such that additional context can be gathered from
`ExecutionConfig` to avoid failing unnecessarily when the file does not
exist.

Closes: astronomer#680
Closes: astronomer#656
arojasb3 pushed a commit to arojasb3/astronomer-cosmos that referenced this issue Jul 14, 2024
…ERNETES` (astronomer#681)

This PR moves validation of the `profiles_yml_path` to later in the dag
generation process such that additional context can be gathered from
`ExecutionConfig` to avoid failing unnecessarily when the file does not
exist.

Closes: astronomer#680
Closes: astronomer#656
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:profile Related to ProfileConfig, like Athena, BigQuery, Clickhouse, Spark, Trino, etc bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant