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

fix: Avoid access to profile when calling str(UnsetProfileConfig) #5209

Merged

Conversation

tomasfarias
Copy link
Contributor

@tomasfarias tomasfarias commented May 3, 2022

resolves #5081

Description

dbt.config.UnsetProfileConfig inherits __str__ from dbt.config.Project. Moreover, UnsetProfileConfig raises an exception when attempting to access unset profile attributes. As Project.__str__ ultimately calls Project.to_project_config and accesses said profile attributes, we override to_project_config in UnsetProfileConfig to avoid accessing the attributes that raise an exception.

This allows calling str(UnsetProfileConfig) without a RuntimeException.

Furthermore, setting both profile fields (profile_name and target_name) to repr=False allow us to call repr(UnsetProfileConfig) without a RuntimeException.

Basic unit testing is also included in commit.

Looks like my IDE also cleaned up some whitespace (runs black on save). I can revert it if needed.

Checklist

@cla-bot
Copy link

cla-bot bot commented May 3, 2022

Thanks for your pull request, and welcome to our community! We require contributors to sign our Contributor License Agreement and we don't seem to have your signature on file. Check out this article for more information on why we have a CLA.

In order for us to review and merge your code, please submit the Individual Contributor License Agreement form attached above above. If you have questions about the CLA, or if you believe you've received this message in error, don't hesitate to ping @drewbanin.

CLA has not been signed by users: @tomasfarias

@tomasfarias
Copy link
Contributor Author

CLA was just signed a minute ago 👍

@cla-bot cla-bot bot added the cla:yes label May 3, 2022
@tomasfarias tomasfarias marked this pull request as ready for review May 3, 2022 17:54
@tomasfarias tomasfarias requested review from a team as code owners May 3, 2022 17:54
dbt.config.UnsetProfileConfig inherits __str__ from
dbt.config.Project. Moreover, UnsetProfileConfig also raises an
exception when attempting to access unset profile attributes. As
Project.__str__ ultimately calls to_project_config and accesses said
profile attributes, we override to_project_config in
UnsetProfileConfig to avoid accessing the attributes that raise an
exception.

This allows calling str(UnsetProfileConfig) and
repr(UnsetProfileConfig).

Basic unit testing is also included in commit.
@tomasfarias tomasfarias force-pushed the implement_str_for_unsetprofileconfig branch from c5301e7 to 097d24a Compare May 3, 2022 18:01
Copy link
Contributor

@ChenyuLInx ChenyuLInx left a comment

Choose a reason for hiding this comment

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

@tomasfarias Thanks for putting up this amazing PR! Looks great to me! I will get the GHA all running and get it merged!
Sorry that it took me a bit to come back to the issue.

@ChenyuLInx ChenyuLInx closed this May 12, 2022
@ChenyuLInx ChenyuLInx reopened this May 12, 2022
@ChenyuLInx ChenyuLInx self-assigned this May 12, 2022
@ChenyuLInx ChenyuLInx merged commit fc1fc2d into dbt-labs:main May 13, 2022
agoblet pushed a commit to BigDataRepublic/dbt-core that referenced this pull request May 20, 2022
…t-labs#5209)

* fix: Avoid access to profile when calling str(UnsetProfileConfig)

dbt.config.UnsetProfileConfig inherits __str__ from
dbt.config.Project. Moreover, UnsetProfileConfig also raises an
exception when attempting to access unset profile attributes. As
Project.__str__ ultimately calls to_project_config and accesses said
profile attributes, we override to_project_config in
UnsetProfileConfig to avoid accessing the attributes that raise an
exception.

This allows calling str(UnsetProfileConfig) and
repr(UnsetProfileConfig).

Basic unit testing is also included in commit.

* fix: Skip repr for profile fields in UnsetProfileConfig

* chore(changie): Add changie file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-501] [Bug] UnsetProfileConfig str and repr raise RuntimeException
2 participants