Introduce specialized registry for Jinja macros#460
Conversation
sqlmesh/dbt/model.py
Outdated
There was a problem hiding this comment.
why do we call a bunch of private methods? maybe lets make them public
There was a problem hiding this comment.
rendered in this case is ModelConfig so it's technically ok.
sqlmesh/utils/jinja.py
Outdated
There was a problem hiding this comment.
make sure this doesn't mess with anywhere that we depend on order, perhaps in hashing etc
There was a problem hiding this comment.
Hm, why do you think we may rely on order for dependencies?
izeigerman
force-pushed
the
better-jinja-macros
branch
from
5df3c91 to
186093e
Compare
examples/sushi_dbt/profiles.yml
Outdated
There was a problem hiding this comment.
won't this mask valid exceptions?
There was a problem hiding this comment.
Tests fail without it. So previously we were just ignoring missing environment variables. Not sure how the current behavior is different from before.
There was a problem hiding this comment.
I think the issue is we are loading all targets and we should only load the one requested.
There was a problem hiding this comment.
Do you think this is something I introduced?
There was a problem hiding this comment.
Ah, doesn't look like it. Looks like Profile.to_sqlmesh intentionally returns all of them.
sqlmesh/dbt/model.py
Outdated
There was a problem hiding this comment.
Should the date_dict be part of builtin_jinja? Or special here?
There was a problem hiding this comment.
It's determined at runtime, so yeah - special.
izeigerman
force-pushed
the
better-jinja-macros
branch
from
e9a9ba9 to
fc81f6d
Compare
izeigerman
force-pushed
the
better-jinja-macros
branch
from
6eb8269 to
bfc734b
Compare
| self._target = value | ||
| if not self.project_name: | ||
| raise ConfigError(f"Must assign project_name before assigning target") | ||
| self._builtins["target"] = self._target.target_jinja(self.project_name) |
4 participants
What hasn't been covered:
adapter.dispatch.