Remove incorrect docstring from DbtLocalBaseOperator #797

jakob-hvitnov-telia · 2024-01-12T12:57:25Z

Remove incorrect docstring from DbtLocalBaseOperator (relates to #796)

netlify · 2024-01-12T12:57:30Z

✅ Deploy Preview for sunny-pastelito-5ecb04 ready!

Name	Link
🔨 Latest commit	`6c89aaf`
🔍 Latest deploy log	https://app.netlify.com/sites/sunny-pastelito-5ecb04/deploys/65afd51b07265600086f861c
😎 Deploy Preview	https://deploy-preview-797--sunny-pastelito-5ecb04.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

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

jbandoro

Thanks for opening this PR!

As mentioned in the comment below emit_datasets is being set to True by default in the base operator that DbtLocalBaseOperator inherits which is causing the unit tests to fail atm.

Though I'm not sure this would fix the issue you're having since emit_datasets=True is currently the default?

cosmos/operators/local.py

jbandoro · 2024-01-12T18:29:49Z

I also noticed that RenderConfig also has emit_datasets and it's used below when instantiating the airflow operators:

astronomer-cosmos/cosmos/converter.py

Lines 246 to 253 in 1b7f4a0

    
           task_args = { 
        
               **operator_args, 
        
               "project_dir": execution_config.project_path, 
        
               "profile_config": profile_config, 
        
               "emit_datasets": render_config.emit_datasets, 
        
               "env": env_vars, 
        
               "vars": dbt_vars, 
        
           }

So any operator_args with emit_datasets will be overrided by what is set in RenderConfig when merged above. Since this is used for all operators I think we have to keep emit_datasets in the base operator.

cosmos/operators/local.py

codecov · 2024-01-15T10:25:03Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (522f64a) 94.69% compared to head (13a2d63) 94.62%.

❗ Current head 13a2d63 differs from pull request most recent head 6c89aaf. Consider uploading reports for the commit 6c89aaf to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #797      +/-   ##
==========================================
- Coverage   94.69%   94.62%   -0.07%     
==========================================
  Files          55       55              
  Lines        2449     2419      -30     
==========================================
- Hits         2319     2289      -30     
  Misses        130      130

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

…rator

tatiana · 2024-01-18T11:45:01Z

@jakob-hvitnov-telia I have the impression our documentation may be incorrect:
https://astronomer.github.io/astronomer-cosmos/configuration/lineage.html#installation

Please, could you confirm if you still face an issue following what I suggested in this comment:
#796 (comment)

…rator

Remove incorrect docstring from `DbtLocalBaseOperator` (relatest to #796) --------- Co-authored-by: Justin Bandoro <79104794+jbandoro@users.noreply.github.com> Co-authored-by: Tatiana Al-Chueyr <tatiana.alchueyr@gmail.com> (cherry picked from commit ef2c7bb)

@jbandoro

Bug fixes * Fix: ensure DbtGraph.update_node_dependency is called for all load methods by @jbandoro in #803 * Fix: ensure operator execute method is consistent across all execution base subclasses by @jbandoro in #805 * Fix custom selector when test node has no depends_on values by @tatiana in #814 * Fix forwarding selectors to test task when using TestBehavior.AFTER_ALL (#816) Others * Docs: Remove incorrect docstring from DbtLocalBaseOperator by @jakob-hvitnov-telia in #797 * Add more logs to troubleshoot custom selector by @tatiana in #809 * Fix OpenLineage integration documentation by @tatiana in #810 * Fix test dependencies after Airflow 2.8 release by @jbandoro and @tatiana in #806 * Use Airflow constraint file for test environment setup by @jbandoro in #812 * pre-commit updates in #799, #807

@jbandoro

Bug fixes * Fix: ensure DbtGraph.update_node_dependency is called for all load methods by @jbandoro in #803 * Fix: ensure operator execute method is consistent across all execution base subclasses by @jbandoro in #805 * Fix custom selector when test node has no depends_on values by @tatiana in #814 * Fix forwarding selectors to test task when using TestBehavior.AFTER_ALL (#816) Others * Docs: Remove incorrect docstring from DbtLocalBaseOperator by @jakob-hvitnov-telia in #797 * Add more logs to troubleshoot custom selector by @tatiana in #809 * Fix OpenLineage integration documentation by @tatiana in #810 * Fix test dependencies after Airflow 2.8 release by @jbandoro and @tatiana in #806 * Use Airflow constraint file for test environment setup by @jbandoro in #812 * pre-commit updates in #799, #807

@jbandoro

Bug fixes * Fix: ensure DbtGraph.update_node_dependency is called for all load methods by @jbandoro in #803 * Fix: ensure operator execute method is consistent across all execution base subclasses by @jbandoro in #805 * Fix custom selector when test node has no depends_on values by @tatiana in #814 * Fix forwarding selectors to test task when using TestBehavior.AFTER_ALL by @tatiana in #816 Others * Docs: Remove incorrect docstring from DbtLocalBaseOperator by @jakob-hvitnov-telia in #797 * Add more logs to troubleshoot custom selector by @tatiana in #809 * Fix OpenLineage integration documentation by @tatiana in #810 * Fix test dependencies after Airflow 2.8 release by @jbandoro and @tatiana in #806 * Use Airflow constraint file for test environment setup by @jbandoro in #812 * pre-commit updates in #799, #807

@jbandoro

**Bug fixes** * Fix: ensure ``DbtGraph.update_node_dependency`` is called for all load methods by @jbandoro in #803 * Fix: ensure operator ``execute`` method is consistent across all execution base subclasses by @jbandoro in #805 * Fix custom selector when ``test`` node has no ``depends_on`` values by @tatiana in #814 * Fix forwarding selectors to test task when using ``TestBehavior.AFTER_ALL`` by @tatiana in #816 **Others** * Docs: Remove incorrect docstring from ``DbtLocalBaseOperator`` by @jakob-hvitnov-telia in #797 * Add more logs to troubleshoot custom selector by @tatiana in #809 * Fix OpenLineage integration documentation by @tatiana in #810 * Fix test dependencies after Airflow 2.8 release by @jbandoro and @tatiana in #806 * Use Airflow constraint file for test environment setup by @jbandoro in #812 * pre-commit updates in #799, #807

@jbandoro

Bug fixes * Fix: ensure DbtGraph.update_node_dependency is called for all load methods by @jbandoro in #803 * Fix: ensure operator execute method is consistent across all execution base subclasses by @jbandoro in #805 * Fix custom selector when test node has no depends_on values by @tatiana in #814 * Fix forwarding selectors to test task when using TestBehavior.AFTER_ALL by @tatiana in #816 Others * Docs: Remove incorrect docstring from DbtLocalBaseOperator by @jakob-hvitnov-telia in #797 * Add more logs to troubleshoot custom selector by @tatiana in #809 * Fix OpenLineage integration documentation by @tatiana in #810 * Fix test dependencies after Airflow 2.8 release by @jbandoro and @tatiana in #806 * Use Airflow constraint file for test environment setup by @jbandoro in #812 * pre-commit updates in #799, #807

@jbandoro

**Bug fixes** * Fix: ensure ``DbtGraph.update_node_dependency`` is called for all load methods by @jbandoro in #803 * Fix: ensure operator ``execute`` method is consistent across all execution base subclasses by @jbandoro in #805 * Fix custom selector when ``test`` node has no ``depends_on`` values by @tatiana in #814 * Fix forwarding selectors to test task when using ``TestBehavior.AFTER_ALL`` by @tatiana in #816 **Others** * Docs: Remove incorrect docstring from ``DbtLocalBaseOperator`` by @jakob-hvitnov-telia in #797 * Add more logs to troubleshoot custom selector by @tatiana in #809 * Fix OpenLineage integration documentation by @tatiana in #810 * Fix test dependencies after Airflow 2.8 release by @jbandoro and @tatiana in #806 * Use Airflow constraint file for test environment setup by @jbandoro in #812 * pre-commit updates in #799, #807 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Justin Bandoro <79104794+jbandoro@users.noreply.github.com> Co-authored-by: Jakob Aron Hvitnov <141235900+jakob-hvitnov-telia@users.noreply.github.com>

Remove incorrect docstring from `DbtLocalBaseOperator` (relatest to astronomer#796) --------- Co-authored-by: Justin Bandoro <79104794+jbandoro@users.noreply.github.com> Co-authored-by: Tatiana Al-Chueyr <tatiana.alchueyr@gmail.com>

@jbandoro

**Bug fixes** * Fix: ensure ``DbtGraph.update_node_dependency`` is called for all load methods by @jbandoro in astronomer#803 * Fix: ensure operator ``execute`` method is consistent across all execution base subclasses by @jbandoro in astronomer#805 * Fix custom selector when ``test`` node has no ``depends_on`` values by @tatiana in astronomer#814 * Fix forwarding selectors to test task when using ``TestBehavior.AFTER_ALL`` by @tatiana in astronomer#816 **Others** * Docs: Remove incorrect docstring from ``DbtLocalBaseOperator`` by @jakob-hvitnov-telia in astronomer#797 * Add more logs to troubleshoot custom selector by @tatiana in astronomer#809 * Fix OpenLineage integration documentation by @tatiana in astronomer#810 * Fix test dependencies after Airflow 2.8 release by @jbandoro and @tatiana in astronomer#806 * Use Airflow constraint file for test environment setup by @jbandoro in astronomer#812 * pre-commit updates in astronomer#799, astronomer#807 --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Justin Bandoro <79104794+jbandoro@users.noreply.github.com> Co-authored-by: Jakob Aron Hvitnov <141235900+jakob-hvitnov-telia@users.noreply.github.com>

Proposed solution

6aaa4c3

jakob-hvitnov-telia requested a review from a team as a code owner January 12, 2024 12:57

jakob-hvitnov-telia requested a review from a team January 12, 2024 12:57

dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Jan 12, 2024

jakob-hvitnov-telia temporarily deployed to external January 12, 2024 12:57 — with GitHub Actions Inactive

dosubot bot added area:datasets Related to the Airflow datasets feature/module execution:local Related to Local execution environment parsing:custom Related to custom parsing, like custom DAG parsing, custom DBT parsing, etc labels Jan 12, 2024

jakob-hvitnov-telia changed the title ~~Proposed solution~~ Proposed solution for #796 Jan 12, 2024

jbandoro reviewed Jan 12, 2024

View reviewed changes

cosmos/operators/local.py Outdated Show resolved Hide resolved

Changes after review

89e1042

jakob-hvitnov-telia temporarily deployed to external January 15, 2024 07:48 — with GitHub Actions Inactive

tatiana reviewed Jan 15, 2024

View reviewed changes

cosmos/operators/local.py Show resolved Hide resolved

Merge branch 'main' into reintroduce_emit_datasets_in_DbtLocalBaseOpe…

13a2d63

…rator

jbandoro temporarily deployed to external January 18, 2024 01:07 — with GitHub Actions Inactive

jbandoro changed the title ~~Proposed solution for #796~~ Fix: remove emit_datasets from DbtLocalBaseOperator docstring Jan 18, 2024

jbandoro added this to the 1.3.2 milestone Jan 18, 2024

tatiana changed the title ~~Fix: remove emit_datasets from DbtLocalBaseOperator docstring~~ Remove incorrect docstring from DbtLocalBaseOperator Jan 23, 2024

Merge branch 'main' into reintroduce_emit_datasets_in_DbtLocalBaseOpe…

6c89aaf

…rator

tatiana temporarily deployed to external January 23, 2024 15:02 — with GitHub Actions Inactive

tatiana approved these changes Jan 23, 2024

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Jan 23, 2024

tatiana merged commit ef2c7bb into astronomer:main Jan 23, 2024
38 of 39 checks passed

tatiana mentioned this pull request Jan 26, 2024

Release 1.3.2 #817

Merged

tatiana mentioned this pull request Jan 27, 2024

Release 1.3 #818

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove incorrect docstring from DbtLocalBaseOperator #797

Remove incorrect docstring from DbtLocalBaseOperator #797

jakob-hvitnov-telia commented Jan 12, 2024 •

edited by tatiana

Loading

netlify bot commented Jan 12, 2024 •

edited

Loading

jbandoro left a comment

jbandoro commented Jan 12, 2024 •

edited

Loading

codecov bot commented Jan 15, 2024 •

edited

Loading

tatiana commented Jan 18, 2024

Remove incorrect docstring from DbtLocalBaseOperator #797

Remove incorrect docstring from DbtLocalBaseOperator #797

Conversation

jakob-hvitnov-telia commented Jan 12, 2024 • edited by tatiana Loading

netlify bot commented Jan 12, 2024 • edited Loading

✅ Deploy Preview for sunny-pastelito-5ecb04 ready!

jbandoro left a comment

Choose a reason for hiding this comment

jbandoro commented Jan 12, 2024 • edited Loading

codecov bot commented Jan 15, 2024 • edited Loading

Codecov Report

tatiana commented Jan 18, 2024

jakob-hvitnov-telia commented Jan 12, 2024 •

edited by tatiana

Loading

netlify bot commented Jan 12, 2024 •

edited

Loading

jbandoro commented Jan 12, 2024 •

edited

Loading

codecov bot commented Jan 15, 2024 •

edited

Loading