Skip to content

config call sites#4169

Merged
emmyoop merged 7 commits intofeature/structured-loggingfrom
er/sl-config
Nov 2, 2021
Merged

config call sites#4169
emmyoop merged 7 commits intofeature/structured-loggingfrom
er/sl-config

Conversation

@emmyoop
Copy link
Member

@emmyoop emmyoop commented Oct 29, 2021

Description

Modifying call sites for structured logging in core/dbt/config

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change

Comment on lines +311 to +321
@dataclass
class ProfileLoadError(ShowException, DebugLevel, CliEventABC):
exc: Exception = Exception('')

def cli_msg(self) -> str:
return f"Profile not loaded due to error: {self.exc}"
Copy link
Member Author

Choose a reason for hiding this comment

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

@nathaniel-may I'm not sure this was the intention or is I'm missing something. Because ShowException has defaults it throws a Fields without default values cannot appear after fields with default values error unless a default is provided for any fields in the type. Is this how we want to handle it? Or is there something I'm missing?

Copy link
Contributor

Choose a reason for hiding this comment

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

I tried to recreate locally, and the code you have here passes mypy checks and runs fine for me when I create an instance of this class. How did you get that exception to throw?

Copy link
Contributor

Choose a reason for hiding this comment

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

@iknox-fa figured it out. The lib we use for 3.6 compat with dataclasses is the issue. We probably will have to set all the values to their defaults again in the subclasses until we drop support for 3.6. Let's TODO setting these defaults to remove them after dropping 3.6.

@emmyoop emmyoop marked this pull request as ready for review November 1, 2021 19:48
@nathaniel-may
Copy link
Contributor

@iknox-fa is going to fix our ShowException issue on feature/structured-logging so let's wait to merge till we can remove the default and use the new ShowException class.

@iknox-fa
Copy link
Contributor

iknox-fa commented Nov 2, 2021

@iknox-fa is going to fix our ShowException issue on feature/structured-logging so let's wait to merge till we can remove the default and use the new ShowException class.

Fixed!

@emmyoop emmyoop merged commit bbdb377 into feature/structured-logging Nov 2, 2021
@emmyoop emmyoop deleted the er/sl-config branch November 2, 2021 16:01
@nathaniel-may nathaniel-may mentioned this pull request Nov 8, 2021
21 tasks
emmyoop added a commit that referenced this pull request Nov 8, 2021
* update config use structured logging

* WIP

* minor cleanup

* fixed merge error

* added in ShowException

* added todo to remove defaults after dropping 3.6

* removed todo that is obsolete
emmyoop added a commit that referenced this pull request Nov 8, 2021
* update config use structured logging

* WIP

* minor cleanup

* fixed merge error

* added in ShowException

* added todo to remove defaults after dropping 3.6

* removed todo that is obsolete
kwigley pushed a commit that referenced this pull request Nov 9, 2021
* update config use structured logging

* WIP

* minor cleanup

* fixed merge error

* added in ShowException

* added todo to remove defaults after dropping 3.6

* removed todo that is obsolete
nathaniel-may pushed a commit that referenced this pull request Nov 9, 2021
* update config use structured logging

* WIP

* minor cleanup

* fixed merge error

* added in ShowException

* added todo to remove defaults after dropping 3.6

* removed todo that is obsolete
iknox-fa pushed a commit that referenced this pull request Feb 8, 2022
* update config use structured logging

* WIP

* minor cleanup

* fixed merge error

* added in ShowException

* added todo to remove defaults after dropping 3.6

* removed todo that is obsolete

automatic commit by git-black, original commits:
  6b36b18
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.

3 participants