Reconcile SourceConfig

[drewbanin]

Describe the feature

The SourceConfig class is complicated, slow, and hard to reason about. Let's replace it with something simpler and more explicitly well-defined.

We should work to ensure that the intended semantics of the class are preserved, but it is acceptable if in this refactor we need to "break" poorly defined or undocumented behaviors.

@beckjake to follow up with specific thoughts about the nature of the code changes required here.

[beckjake]

Some key things we want to add from an interface perspective:

• types. Internally-specified config values, including macros, should have types assigned and dbt should make sure the output matches and coerce it otherwise. enabled: bool should mean that the type of anything passed to enabled is converted to a bool (and dbt should fail with an error if it can't be converted). User-supplied configs should just have a type of Any.
• add get_{type}methods. get() will return the value as-stored like it currently does, but we should also provide:
  • get_bool -> convert to bool or error
  • get_int -> convert to int or error
  • get_str -> str (can't see how this could fail!)
  • get_list -> convert to list or error
• support both dbt_project.yml configs and config() calls
  • Users should be able to configure everything with this object, and sources should just be special in the sense that they never happen to call config().

My current thinking is that we'd also generate a separate type entirely that handles the config value available in the context. That type might be really similar to/internally own a SourceConfig, but I don't think we necessarily need to conflate the part that says "what is the correct value for this key in this context" and the part that says "here is a config value I want to set in parsing". Currently I think that's where a lot of SourceConfig's complexity lives.

An interesting related question is, what should this do?:
dbt_project.yml:

models:
  my_project:
    my_key: foo

models/my_model.sql:

{% set value = config.require('my_key') %}
{% if value == 'foo' %}
    config.set('my_key', 'bar')
{% else %}
    config.set('my_key', 'foo')
{% endif %}
select {{ config.require('my_key') }} as value

> select value from my_model;
????

I think there's a few ways to interpret this:

1. at parse-time, the control flow set my_key to 'bar', and then it was locked in and set calls were ignored at runtime, so that's what it should show.
2. at parse-time, the control flow set my_key to 'bar', but then at run-time, the control flow set it back to 'foo', so that's what it should show.
3. at parse-time, set calls were ignored, and at run-time set my_key to 'bar', so that's what it should show.
4. this should be an error, don't do that (I think this is both the wrong answer and hard to implement)

I like 1 the best, though it has some ugly caveats. In particular, doing config.set(my_key, adapter.execute(), which feels like it should work, does not.
I think 2 is almost certainly wrong. I would feel very bad about doing that.
While 3 solves the problem I described for 1, it means config.set("materalized", "table") and config(...) are different, which seems wrong!
4 is both probably wrong and a pain to implement.

For whatever it's worth, I couldn't even tell you what SourceConfig currently does with this sample without looking at it. I like to think 1.

[drewbanin]

Let's spend ~2 days doing a proof of concept here. @beckjake to experiment with a rough approach and @drewbanin to review. We'll figure out how to proceed with more discrete issues from there

[drewbanin]

@drewbanin to make an issue detailing how config() should work at runtime (eg. using a ref as a config:

{{ config(alias=ref('other').identifier) }}

This does the wrong thing today!

Drew and Jake to figure out what the right way forwards is to support this use case

[drewbanin]

closed by #2312