dbt-core
dbt-core copied to clipboard
[CT-756] Stringify user-provided messages to `{{ log() }}`
Reproduction case
-- models/any_model.sql
{% set some_list = ['a', 'b', 'c'] %}
{{ log(some_list, info = true) }}
select 1 as id
$ DBT_ENV_SECRET_WHATEVER=1234 dbt run
12:12:30 Running with dbt=1.2.0-a1
12:12:30 Encountered an error:
'list' object has no attribute 'replace'
I'm not even using the secret, but because I've defined one, dbt is searching log messages to scrub out any instances of the secret value. This error is not related to the recent change re: secret rendering. It's just related to how we're scrubbing secrets from log messages in general, and have been since v1.0.0:
https://github.com/dbt-labs/dbt-core/blob/6e8388c653c6bbeee934aea7db93730bd88df091/core/dbt/events/functions.py#L122-L128
When a user passes a non-string
object into {{ log() }}
, we end up passing that object straight through to scrub_secrets
. Which expects to be receiving a string!
The type signature of log
actually expects to be receiving a string, too:
https://github.com/dbt-labs/dbt-core/blob/6e8388c653c6bbeee934aea7db93730bd88df091/core/dbt/context/base.py#L543-L562
Resolution
We could put this logic anywhere, but it feels like the simplest and most appropriate fix may well be to stringify whatever users are passing into log
, before firing it:
def log(msg: str, info: bool = False) -> str:
...
# user-provided msg should be a string
msg = str(msg)
if info:
fire_event(MacroEventInfo(msg=msg))
else:
fire_event(MacroEventDebug(msg=msg))
return ""
This successfully handles some pretty weird edge cases:
{{ log(this, info = true) }}
{{ log(create_table_as, info = true) }}
{{ log(context, info = true) }}
12:11:26 "jerco"."dbt_jcohen"."model_a"
12:11:26 <dbt.clients.jinja.MacroGenerator object at 0x10dca87f0>
... giant context blob ...
Still, we should make sure it raises a useful error, in case users try passing in objects that aren't string serializable. (I can't think of an example right now, but maybe there is one?)
@VersusFacit does this seem related to the ticket you just closed out? Should we solve this further down the stack? #5436 #5874
Solution provided in this PR