Skip to content
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

Show the group when a private model is referenced #59

Closed
noel opened this issue May 11, 2024 · 6 comments
Closed

Show the group when a private model is referenced #59

noel opened this issue May 11, 2024 · 6 comments
Assignees
Labels
enhancement New feature or request question Further information is requested triage This issue is being investigated
Milestone

Comments

@noel
Copy link

noel commented May 11, 2024

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

@noel noel added enhancement New feature or request triage This issue is being investigated labels May 11, 2024
@nicholasyager
Copy link
Owner

nicholasyager commented May 11, 2024

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.

@nicholasyager nicholasyager added question Further information is requested triage This issue is being investigated and removed triage This issue is being investigated labels May 11, 2024
@z3z1ma
Copy link

z3z1ma commented May 12, 2024

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
Copy link
Owner

🤔 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
Copy link
Owner

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
Copy link

z3z1ma commented May 13, 2024

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
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested triage This issue is being investigated
Projects
None yet
Development

No branches or pull requests

3 participants