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

Move DbtRunner related functions into dbt/runner.py module #1480

Merged
merged 11 commits into from
Jan 23, 2025

Conversation

tatiana
Copy link
Collaborator

@tatiana tatiana commented Jan 21, 2025

Previously, @jbandoro added support for Cosmos to use dbtRunner to execute dbt commands in the Airflow worker nodes when using ExecutionMode.LOCAL instead of Python's subprocess #850. This change significantly improved Cosmos resource utilisation in worker nodes.

The first step to implementing support to use dbtRunner during DAG parsing (ticket #865) when using LoadMode.DBT_LS is to extract dbtRunner methods somewhere it can be reused by graph.py and is not specific to operators.py.

This PR does the following:

  • Create dbt/runner.py
  • Move all dbtRunnerrelated functions to this module, refactoring them as needed
  • Refactor operators/local.py to use the newly introduced methods

Related: #865

Copy link

netlify bot commented Jan 21, 2025

Deploy Preview for sunny-pastelito-5ecb04 ready!

Name Link
🔨 Latest commit 006cc1d
🔍 Latest deploy log https://app.netlify.com/sites/sunny-pastelito-5ecb04/deploys/6792172380736100089bd903
😎 Deploy Preview https://deploy-preview-1480--sunny-pastelito-5ecb04.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

cloudflare-workers-and-pages bot commented Jan 21, 2025

Deploying astronomer-cosmos with  Cloudflare Pages  Cloudflare Pages

Latest commit: 006cc1d
Status: ✅  Deploy successful!
Preview URL: https://6ef0ef85.astronomer-cosmos.pages.dev
Branch Preview URL: https://issue-865-refactor.astronomer-cosmos.pages.dev

View logs

@tatiana tatiana changed the title WIP: Refactor DbtRunner utility functions into dbt/runner.py module Extract DbtRunner-related functions into dbt/runner.py module Jan 21, 2025
@tatiana tatiana marked this pull request as ready for review January 21, 2025 17:03
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. area:parsing Related to parsing DAG/DBT improvement, issues, or fixes execution:local Related to Local execution environment parsing:dbt_ls Issues, questions, or features related to dbt_ls parsing labels Jan 21, 2025
@tatiana tatiana changed the title Extract DbtRunner-related functions into dbt/runner.py module Move DbtRunner related functions into dbt/runner.py module Jan 21, 2025
Copy link
Contributor

@pankajkoti pankajkoti left a comment

Choose a reason for hiding this comment

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

LGTM. Great refactor and organization! I’ve added a few minor questions and suggestions in-line.

cosmos/dbt/runner.py Outdated Show resolved Hide resolved
cosmos/dbt/runner.py Outdated Show resolved Hide resolved
cosmos/operators/local.py Outdated Show resolved Hide resolved
cosmos/operators/local.py Show resolved Hide resolved
cosmos/operators/local.py Show resolved Hide resolved
Copy link

codecov bot commented Jan 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.60%. Comparing base (74c7c7d) to head (006cc1d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1480      +/-   ##
==========================================
+ Coverage   96.57%   96.60%   +0.03%     
==========================================
  Files          73       74       +1     
  Lines        4374     4414      +40     
==========================================
+ Hits         4224     4264      +40     
  Misses        150      150              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tatiana tatiana merged commit 692314f into main Jan 23, 2025
69 checks passed
@tatiana tatiana deleted the issue-865-refactor branch January 23, 2025 11:07
@tatiana tatiana added this to the Cosmos 1.9.0 milestone Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:parsing Related to parsing DAG/DBT improvement, issues, or fixes execution:local Related to Local execution environment parsing:dbt_ls Issues, questions, or features related to dbt_ls parsing size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants