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

update graph parsing mode when virtualenv is selected #402

Merged
merged 7 commits into from
Jul 27, 2023

Conversation

CorsettiS
Copy link
Contributor

@CorsettiS CorsettiS commented Jul 27, 2023

Description

When using a virtualenv to run our dbt tasks, we are working under the hypothesis that dbt is not installed in the machine. However as of now, if we choose virtualenv as the execution mode, dbt tries to parse the graph of the dbt tasks with load_via_dbt_ls(), while it should point out to load_via_custom_parser() due to the fact aforementioned. we can accomplish it by removing the virutalenv executor from the elif condition for load_via_dbt_ls(), so it will be picked up automatically by the else condition.

Related Issue(s)

closes #401

Breaking Change?

No. it worked fine in the dev sandbox, although I did not have the opportunity to test it with any image without dbt pre-installed.
Obs: I had to change also the tests for this 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

@CorsettiS CorsettiS requested a review from a team as a code owner July 27, 2023 11:30
@CorsettiS CorsettiS requested a review from a team July 27, 2023 11:30
@netlify
Copy link

netlify bot commented Jul 27, 2023

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name Link
🔨 Latest commit d109842
🔍 Latest deploy log https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/64c29065e15b850008cd158e

@CorsettiS CorsettiS changed the title Update graph.py update graph parsing mode when virtualenv is selected Jul 27, 2023
@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (2265e16) 79.36% compared to head (d109842) 79.36%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #402   +/-   ##
=======================================
  Coverage   79.36%   79.36%           
=======================================
  Files          48       48           
  Lines        1691     1691           
=======================================
  Hits         1342     1342           
  Misses        349      349           
Files Changed Coverage Δ
cosmos/dbt/graph.py 75.22% <100.00%> (ø)

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

Copy link
Collaborator

@jlaneve jlaneve left a comment

Choose a reason for hiding this comment

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

nice, thank you! Integration tests are failing because external contributors don't have access to GH actions secrets, but this looks good

@jlaneve jlaneve merged commit ca20b79 into astronomer:main Jul 27, 2023
tatiana pushed a commit that referenced this pull request Aug 9, 2023
## Description

When using a virtualenv to run our dbt tasks, we are working under the
hypothesis that dbt is not installed in the machine. However as of now,
if we choose virtualenv as the execution mode, dbt tries to parse the
graph of the dbt tasks with **load_via_dbt_ls()**, while it should point
out to **load_via_custom_parser()** due to the fact aforementioned. we
can accomplish it by removing the virutalenv executor from the _elif_
condition for **load_via_dbt_ls()**, so it will be picked up
automatically by the _else_ condition.

## Related Issue(s)

closes #401

## Breaking Change?

No. it worked fine in the dev sandbox, although I did not have the
opportunity to test it with any image without dbt pre-installed.
Obs: I had to change also the tests for this change

## Checklist

- [X] 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

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Julian LaNeve <lanevejulian@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to find the dbt executable while trying to run cosmos with virtual env
2 participants