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

Add default source nodes rendering #661

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

arojasb3
Copy link

@arojasb3 arojasb3 commented Nov 9, 2023

Description

I'm aiming to give a default behaviour to source nodes for checking their source freshness and their tests.
One of the main limitations I found while using the custom_callback functions on source nodes to check freshness is that those node were being created to 100% of sources but not all of them required freshness checks, this made workers waste compute time.

I'm adding a new variable into the DbtNode class called has_freshness which would be True for sources with freshness checks and False for the ones that not and any other resource type.

All sources with the has_freshness == False will be rendered as Empty Operators, to keep the dbt's behavior of showing sources as suggested in issue #630.

Related Issue(s)

#630

Breaking Change?

Checklist

  • I have made corresponding changes to the documentation (if required)
  • I have added tests that prove my fix is effective or that my feature works

Copy link

netlify bot commented Nov 9, 2023

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name Link
🔨 Latest commit 99d4be7
🔍 Latest deploy log https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/654c55a5e83e2700089f7f86

task_full_name = node.unique_id[len("source.") :]
task_id = f"{task_full_name}_source"
args["select"] = f"source:{node.unique_id[len('source.'):]}"
args["models"] = None
Copy link
Author

@arojasb3 arojasb3 Nov 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tatiana I have a feeling that this lines can be written somewhere else.
My dbt testing showed that
dbt source freshness --models [any] fails, while using the --select works. I ended up manually setting as None the models argument.
Any thoughts?

if node.has_freshness is False:
return TaskMetadata(
id=task_id,
# arguments=args,
Copy link
Author

@arojasb3 arojasb3 Nov 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tatiana Returning an EmptyOperator just seemed "grayish" in the dag UI. Looking at all the cool cosmos-related operators "DbtLocal..." "Dbt...", etc and having an EmptyOperator didn't seemed to go with the "cosmos pattern", Should we create a dummySourceOperator that inherits EmptyOperator just for the sake of having all Operators follow the same naming pattern?

@@ -742,3 +742,15 @@ def __init__(self, **kwargs: str) -> None:
raise DeprecationWarning(
"The DbtDepsOperator has been deprecated. " "Please use the `install_deps` flag in dbt_args instead."
)


class DbtSourceLocalOperator(DbtLocalBaseOperator):
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tatiana

Since #655 also creates a pretty similar Operator, I can wait until that is merged to follow the same naming pattern.

I tried using the DbtSourceLocalOperator name for this function to work.

@tatiana tatiana added the area:rendering Related to rendering, like Jinja, Airflow tasks, etc label Nov 9, 2023
@tatiana tatiana added the status:awaiting-reviewer The issue/PR is awaiting for a reviewer input label Dec 14, 2023
@tatiana tatiana added this to the 1.6.0 milestone Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:rendering Related to rendering, like Jinja, Airflow tasks, etc status:awaiting-reviewer The issue/PR is awaiting for a reviewer input
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants