nicholasyager / dbt-loom

A dbt-core plugin to weave together multi-project dbt-core deployments
The Unlicense
104 stars 19 forks source link

Show the group when a private model is referenced #59

Closed noel closed 3 months ago

noel commented 3 months ago

Is your feature request related to a problem? Please describe. PR 40 shows better error when accessing a private or protected model of the upstream project, however, the group is missing from the message shown.

Node model.great_bay.test_balboa_models attempted to reference node model.balboa.us_population, which is not allowed because the referenced node is private to the '' group.

Describe the solution you'd like Ideally the group of the upstream model is shown in the error like:

Node model.great_bay.test_balboa_models attempted to reference node model.balboa.us_population, which is not allowed because the referenced node is private to the 'marketing' group.

Describe alternatives you've considered None

Additional context None

nicholasyager commented 3 months ago

Thanks for the report @noel! Can you please provide more details about your specific setup that is generating this error?

I ask since, under typical operation, your projects should be hitting a project-level DbtReferenceError during compilation before hitting a group-level DbtReferenceError during run-time. That is to say that compilation should fail due to model being private way before the runtime fails due to a group ownership mismatch.

Additionally, it may be worth pointing out that within dbt-core groups are bounded within a project. All groups have a namespaced UniqueId of the form group.{{project}}.{{group_name}}, so any cross-project group checks, if they were to occur, should always fail without non-trivial modification to dbt-core.

z3z1ma commented 3 months ago

Hey @nicholasyager! 👋

I follow what your saying I think. But can you share what the expected error would be? All that should be required is selecting from a private model in another project.

nicholasyager commented 3 months ago

🤔 There's a good chance that I'm misinformed here. I'll do some digging in the dbt-core source and figure out what I'm missing/not up-to-date on.

nicholasyager commented 3 months ago

Yep, I was out of date here on two(!) counts:

  1. The group check occurs during initial Manifest parsing, which means a non-restricted project can and will evaluate group access during compilation.
  2. Groups are not scoped to a project (despite the creation of unique_ids in a manifest), which means groups can be evaluated in a cross-project manner. It's actually kinda funny how naive this mechanism actually is.

So, we can get this up and running by doing a similar workaround that was performed in #40: Create a function to wrap the ModelNode.from_args class method that injects the appropriate group value into the ModelNode.

It's actually quite interesting that dbt-core doesn't support this already -- it suggests to me that dbt Mesh as implemented by dbt Labs must very limited in scope: only models, with no group checking or any other access controls. Wild.

In any case, this should be a quick feature to implement.

z3z1ma commented 3 months ago

Thanks for digging in, I had strong suspicion when I saw ModelNodeArgs in dbt core code that the issue was clearly that they werent including group in that data structure. Which BLEW my mind. Like it doesn't take a lot of foresight to see that might be absolutely useful to include in cross project interface. 🤦 I digress for the last time though. I am spoiled by the sqlmesh codebase these days.

Appreciate the great plugin. Thanks again.

nicholasyager commented 3 months ago

Resolved by #60. This will go out in the next release.