-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[CT-1783] [Bug] AttributeError: 'dict' object has no attribute 'replace'
when logging a dict if an DBT_ENV_SECRET_
env var is set
#6568
Comments
AttributeError: 'dict' object has no attribute 'replace'
when logging a dict if an DBT_ENV_SECRET_
env var is setAttributeError: 'dict' object has no attribute 'replace'
when logging a dict if an DBT_ENV_SECRET_
env var is set
An idea to fix thisI didn't actually test this out, but I suspect that changing An alternative might be changing Or we could do both! In the big-picture, there's very few Functional testingIt looks like it would be pretty easy to convert @jeremyyeo's reproduction case into a functional test to prevent an accidental regression. |
Any updates on this ?
I was trying to perform a run-operation and this error popped up. I removed all the DBT_ENV vars and rerun and it worked just fine. The original macro being called had a legacy logging step, which queried the database, fetched the values and then printed'em out. After removing the logging, the macro ran just fine. |
@lsabreu96 Glad you found a workaround! We don't have any updates on when a fix for this will be released. But I think there's a couple ways to solve it (neither of which I have actually tested):
(We could do both.) |
This seems like the Correct approach to me, given the promised return type of this function: dbt-core/core/dbt/context/base.py Line 544 in 066346f
If users want the ability to log more structured data (not just a string), that's a different feature request: |
I ran into a related error in the protobuf branch and put a "str" cast around the "msg" in scrub_secrets. So that might have fixed this. In a more general sense, the "msg" in exceptions should probably always be a string... |
[CT-1783](#6568) describes a bug in dbt-core 1.0-1.3 wherein when a `DBT_ENV_SECRET_` all `{{ log("logging stuff", info=True) }}` invocations break. This commit adds a test for this for two reasons: 1. Ensure we don't regress to this behavior going forward 2. Act as a base commit for making the backport fixes to dbt-core 1.0-1.3
…ing (#7249) * Add tests for logging jinja2.Undefined objects [CT-2259](#7108) identifies an issue wherein dbt-core 1.0-1.3 raise errors if a jinja2.Undefined object is attempted to be logged. This generally happened in the form of `{{ log(undefined_variable, info=True) }}`. This commit adding this test exists for two reasons 1. Ensure we don't have a regression in this going forward 2. Exist as a commit to be used for backport fixes for dbt-core 1.0-1.3 * Add tests for checking `DBT_ENV_SECRET_`s don't break logging [CT-1783](#6568) describes a bug in dbt-core 1.0-1.3 wherein when a `DBT_ENV_SECRET_` all `{{ log("logging stuff", info=True) }}` invocations break. This commit adds a test for this for two reasons: 1. Ensure we don't regress to this behavior going forward 2. Act as a base commit for making the backport fixes to dbt-core 1.0-1.3 * Add tests ensuring failed event serialization is handled correctly [CT-2264](#7113) states that failed serialization should result in an exception handling path which will fire another event instead of raising an exception. This is hard to test perfectly because the exception handling path for serialization depending on whether pytest is present. If pytest isn't present, a new event documentation the failed serialization is fired. If pytest is present, the failed serialization gets raised as an exception. Thus this added test ensures that the expected exception is raised and assumes that the correct event will be fired normally. * Log warning when event serialization fails in `msg_to_dict` This commit updates the `msg_to_dict` exception handling path to fire a warning level event instead of raising an exception. Truthfully, we're not sure if this exception handling path is even possible to hit. That's because we recently switched from betterproto to google's protobuf. However, exception path is the subject of [CT-2264](#7113). Though we don't think it's actually possible to hit it anymore, we still want to handle the case if it is. * Update serialization failure note to be a warn level event in `BaseEvent` [CT-2264](#7113) wants logging messages about event serialization failure to be `WARNING` level events. This does that. * Add changie info for changes * Add test to check exception handling of `msg_to_dict`
…ing (#7249) * Add tests for logging jinja2.Undefined objects [CT-2259](#7108) identifies an issue wherein dbt-core 1.0-1.3 raise errors if a jinja2.Undefined object is attempted to be logged. This generally happened in the form of `{{ log(undefined_variable, info=True) }}`. This commit adding this test exists for two reasons 1. Ensure we don't have a regression in this going forward 2. Exist as a commit to be used for backport fixes for dbt-core 1.0-1.3 * Add tests for checking `DBT_ENV_SECRET_`s don't break logging [CT-1783](#6568) describes a bug in dbt-core 1.0-1.3 wherein when a `DBT_ENV_SECRET_` all `{{ log("logging stuff", info=True) }}` invocations break. This commit adds a test for this for two reasons: 1. Ensure we don't regress to this behavior going forward 2. Act as a base commit for making the backport fixes to dbt-core 1.0-1.3 * Add tests ensuring failed event serialization is handled correctly [CT-2264](#7113) states that failed serialization should result in an exception handling path which will fire another event instead of raising an exception. This is hard to test perfectly because the exception handling path for serialization depending on whether pytest is present. If pytest isn't present, a new event documentation the failed serialization is fired. If pytest is present, the failed serialization gets raised as an exception. Thus this added test ensures that the expected exception is raised and assumes that the correct event will be fired normally. * Log warning when event serialization fails in `msg_to_dict` This commit updates the `msg_to_dict` exception handling path to fire a warning level event instead of raising an exception. Truthfully, we're not sure if this exception handling path is even possible to hit. That's because we recently switched from betterproto to google's protobuf. However, exception path is the subject of [CT-2264](#7113). Though we don't think it's actually possible to hit it anymore, we still want to handle the case if it is. * Update serialization failure note to be a warn level event in `BaseEvent` [CT-2264](#7113) wants logging messages about event serialization failure to be `WARNING` level events. This does that. * Add changie info for changes * Add test to check exception handling of `msg_to_dict`
…ing (#7249) * Add tests for logging jinja2.Undefined objects [CT-2259](#7108) identifies an issue wherein dbt-core 1.0-1.3 raise errors if a jinja2.Undefined object is attempted to be logged. This generally happened in the form of `{{ log(undefined_variable, info=True) }}`. This commit adding this test exists for two reasons 1. Ensure we don't have a regression in this going forward 2. Exist as a commit to be used for backport fixes for dbt-core 1.0-1.3 * Add tests for checking `DBT_ENV_SECRET_`s don't break logging [CT-1783](#6568) describes a bug in dbt-core 1.0-1.3 wherein when a `DBT_ENV_SECRET_` all `{{ log("logging stuff", info=True) }}` invocations break. This commit adds a test for this for two reasons: 1. Ensure we don't regress to this behavior going forward 2. Act as a base commit for making the backport fixes to dbt-core 1.0-1.3 * Add tests ensuring failed event serialization is handled correctly [CT-2264](#7113) states that failed serialization should result in an exception handling path which will fire another event instead of raising an exception. This is hard to test perfectly because the exception handling path for serialization depending on whether pytest is present. If pytest isn't present, a new event documentation the failed serialization is fired. If pytest is present, the failed serialization gets raised as an exception. Thus this added test ensures that the expected exception is raised and assumes that the correct event will be fired normally. * Log warning when event serialization fails in `msg_to_dict` This commit updates the `msg_to_dict` exception handling path to fire a warning level event instead of raising an exception. Truthfully, we're not sure if this exception handling path is even possible to hit. That's because we recently switched from betterproto to google's protobuf. However, exception path is the subject of [CT-2264](#7113). Though we don't think it's actually possible to hit it anymore, we still want to handle the case if it is. * Update serialization failure note to be a warn level event in `BaseEvent` [CT-2264](#7113) wants logging messages about event serialization failure to be `WARNING` level events. This does that. * Add changie info for changes * Add test to check exception handling of `msg_to_dict`
…ing (#7249) * Add tests for logging jinja2.Undefined objects [CT-2259](#7108) identifies an issue wherein dbt-core 1.0-1.3 raise errors if a jinja2.Undefined object is attempted to be logged. This generally happened in the form of `{{ log(undefined_variable, info=True) }}`. This commit adding this test exists for two reasons 1. Ensure we don't have a regression in this going forward 2. Exist as a commit to be used for backport fixes for dbt-core 1.0-1.3 * Add tests for checking `DBT_ENV_SECRET_`s don't break logging [CT-1783](#6568) describes a bug in dbt-core 1.0-1.3 wherein when a `DBT_ENV_SECRET_` all `{{ log("logging stuff", info=True) }}` invocations break. This commit adds a test for this for two reasons: 1. Ensure we don't regress to this behavior going forward 2. Act as a base commit for making the backport fixes to dbt-core 1.0-1.3 * Log warning when event serialization fails in `msg_to_dict` This commit updates the `msg_to_dict` exception handling path to fire a warning level event instead of raising an exception. Truthfully, we're not sure if this exception handling path is even possible to hit. That's because we recently switched from betterproto to google's protobuf. However, exception path is the subject of [CT-2264](#7113). Though we don't think it's actually possible to hit it anymore, we still want to handle the case if it is. * Add changie info for changes * Add test to check exception handling of `msg_to_dict`
[CT-1783](#6568) describes a bug in dbt-core 1.0-1.3 wherein when a `DBT_ENV_SECRET_` all `{{ log("logging stuff", info=True) }}` invocations break. This commit adds a test for this for two reasons: 1. Ensure we don't regress to this behavior going forward 2. Act as a base commit for making the backport fixes to dbt-core 1.0-1.3
[CT-1783](#6568) describes a bug in dbt-core 1.0-1.3 wherein when a `DBT_ENV_SECRET_` all `{{ log("logging stuff", info=True) }}` invocations break. This commit adds a test for this for two reasons: 1. Ensure we don't regress to this behavior going forward 2. Act as a base commit for making the backport fixes to dbt-core 1.0-1.3
[CT-1783](#6568) describes a bug in dbt-core 1.0-1.3 wherein when a `DBT_ENV_SECRET_` all `{{ log("logging stuff", info=True) }}` invocations break. This commit adds a test for this for two reasons: 1. Ensure we don't regress to this behavior going forward 2. Act as a base commit for making the backport fixes to dbt-core 1.0-1.3
[CT-1783](#6568) describes a bug in dbt-core 1.0-1.3 wherein when a `DBT_ENV_SECRET_` all `{{ log("logging stuff", info=True) }}` invocations break. This commit adds a test for this for two reasons: 1. Ensure we don't regress to this behavior going forward 2. Act as a base commit for making the backport fixes to dbt-core 1.0-1.3
[CT-1783](#6568) describes a bug in dbt-core 1.0-1.3 wherein when a `DBT_ENV_SECRET_` all `{{ log("logging stuff", info=True) }}` invocations break. This commit adds a test for this for two reasons: 1. Ensure we don't regress to this behavior going forward 2. Act as a base commit for making the backport fixes to dbt-core 1.0-1.3
* Add tests for logging jinja2.Undefined objects [CT-2259](#7108) identifies an issue wherein dbt-core 1.0-1.3 raise errors if a jinja2.Undefined object is attempted to be logged. This generally happened in the form of `{{ log(undefined_variable, info=True) }}`. This commit adding this test exists for two reasons 1. Ensure we don't have a regression in this going forward 2. Exist as a commit to be used for backport fixes for dbt-core 1.0-1.3 * Add tests for checking `DBT_ENV_SECRET_`s don't break logging [CT-1783](#6568) describes a bug in dbt-core 1.0-1.3 wherein when a `DBT_ENV_SECRET_` all `{{ log("logging stuff", info=True) }}` invocations break. This commit adds a test for this for two reasons: 1. Ensure we don't regress to this behavior going forward 2. Act as a base commit for making the backport fixes to dbt-core 1.0-1.3 * Log warning when event serialization fails in `msg_to_dict` This commit updates the `msg_to_dict` exception handling path to fire a warning level event instead of raising an exception. * Add changie info for changes * Add test to check exception handling of `msg_to_dict` * Ensure all events with self.msg stringify self.msg --------- Co-authored-by: leahwicz <[email protected]>
* Add tests for logging jinja2.Undefined objects [CT-2259](#7108) identifies an issue wherein dbt-core 1.0-1.3 raise errors if a jinja2.Undefined object is attempted to be logged. This generally happened in the form of `{{ log(undefined_variable, info=True) }}`. This commit adding this test exists for two reasons 1. Ensure we don't have a regression in this going forward 2. Exist as a commit to be used for backport fixes for dbt-core 1.0-1.3 * Add tests for checking `DBT_ENV_SECRET_`s don't break logging [CT-1783](#6568) describes a bug in dbt-core 1.0-1.3 wherein when a `DBT_ENV_SECRET_` all `{{ log("logging stuff", info=True) }}` invocations break. This commit adds a test for this for two reasons: 1. Ensure we don't regress to this behavior going forward 2. Act as a base commit for making the backport fixes to dbt-core 1.0-1.3 * Add changie info for changes * Ensure `log()` calls from jinja ensure the msg is stringified The simplest way to resolve [CT-2259](#7108) and [CT-1783](#6568) in backports to dbt-core < 1.4 is to ensure `msg` in `BaseContext.log()` is stringified. --------- Co-authored-by: leahwicz <[email protected]>
* Add tests for logging jinja2.Undefined objects [CT-2259](#7108) identifies an issue wherein dbt-core 1.0-1.3 raise errors if a jinja2.Undefined object is attempted to be logged. This generally happened in the form of `{{ log(undefined_variable, info=True) }}`. This commit adding this test exists for two reasons 1. Ensure we don't have a regression in this going forward 2. Exist as a commit to be used for backport fixes for dbt-core 1.0-1.3 * Add tests for checking `DBT_ENV_SECRET_`s don't break logging [CT-1783](#6568) describes a bug in dbt-core 1.0-1.3 wherein when a `DBT_ENV_SECRET_` all `{{ log("logging stuff", info=True) }}` invocations break. This commit adds a test for this for two reasons: 1. Ensure we don't regress to this behavior going forward 2. Act as a base commit for making the backport fixes to dbt-core 1.0-1.3 * Add changie info for changes * Ensure `log()` calls from jinja ensure the msg is stringified The simplest way to resolve [CT-2259](#7108) and [CT-1783](#6568) in backports to dbt-core < 1.4 is to ensure `msg` in `BaseContext.log()` is stringified. --------- Co-authored-by: leahwicz <[email protected]>
* Add tests for logging jinja2.Undefined objects [CT-2259](#7108) identifies an issue wherein dbt-core 1.0-1.3 raise errors if a jinja2.Undefined object is attempted to be logged. This generally happened in the form of `{{ log(undefined_variable, info=True) }}`. This commit adding this test exists for two reasons 1. Ensure we don't have a regression in this going forward 2. Exist as a commit to be used for backport fixes for dbt-core 1.0-1.3 * Add tests for checking `DBT_ENV_SECRET_`s don't break logging [CT-1783](#6568) describes a bug in dbt-core 1.0-1.3 wherein when a `DBT_ENV_SECRET_` all `{{ log("logging stuff", info=True) }}` invocations break. This commit adds a test for this for two reasons: 1. Ensure we don't regress to this behavior going forward 2. Act as a base commit for making the backport fixes to dbt-core 1.0-1.3 * Add changie info for changes * Ensure `log()` calls from jinja ensure the msg is stringified The simplest way to resolve [CT-2259](#7108) and [CT-1783](#6568) in backports to dbt-core < 1.4 is to ensure `msg` in `BaseContext.log()` is stringified.
I see that this issue is fixed, but I had a similar problem that might be related somehow. I got a compilation error when running 'dbt test' in my project saying
for all my unit tests using 'expect()', which was really confusing since the test did not have any except-clause but only expect. Used quite some time to debug what was happening. Came across this issue and thought maybe it had something to do with my environment variable, so I added again to my .zshrc-file (deleted the old line and rewrote again) and ran in terminal
and somehow that fixed it 🤷♀️ |
Is this a new bug in dbt-core?
Current Behavior
If you set a secret env var
DBT_ENV_SECRET_x
and try to log a dict (not related to the env var at all), you run into an error:Expected Behavior
No errors when logging dicts even with secret env vars.
Steps To Reproduce
Relevant log output
Environment
Which database adapter are you using with dbt?
snowflake
Additional Context
We should stringify
scrubbed
.The text was updated successfully, but these errors were encountered: