Make the arg dbt_project_path in the ProjectConfig optional #581

MrBones757 · 2023-10-09T08:55:31Z

Description

This change allows the dbt_project_path argument from ProjectConfig to be optional, and adds the ability to provide manifest_path alone. It also adds the ability for the user to (optionally) define project_name when dbt_project_path is defined, and requires project_name to be defined when dbt_project_path is not defined

Related Issue(s)

closes #569

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

netlify · 2023-10-09T08:55:36Z

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name	Link
🔨 Latest commit	`d5c8449`
🔍 Latest deploy log	https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/65293880c910ea0008c07b6e

codecov · 2023-10-09T11:04:06Z

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (e80abd7) 93.09% compared to head (d5c8449) 93.13%.
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #581      +/-   ##
==========================================
+ Coverage   93.09%   93.13%   +0.04%     
==========================================
  Files          51       51              
  Lines        2041     2053      +12     
==========================================
+ Hits         1900     1912      +12     
  Misses        141      141

Files	Coverage Δ
cosmos/config.py	`93.75% <100.00%> (+0.80%)`	⬆️
cosmos/operators/local.py	`90.90% <100.00%> (+0.02%)`	⬆️

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

MrBones757 · 2023-10-10T07:29:16Z

hello @tatiana
Thanks for your feedback on slack.

I have this MR to a point where i think it is just about ready to go, and would like to seek some review from you and / or your team.

I have read though the contributing guide, and believe i've done whats needed based on my understanding.
As far as local testing goes:

The version of code currently pushed to this MR has passed all coverage tests (hatch run tests:test-cov)
I have also attempted to run the integration tests locally, most of which pass with the exception of the databricks test because i don't have that setup locally (ill check that one against CI)

I think the only item left here is to complete the docs updates, however before i complete that i want to make sure the logic is acceptable. Please let me know if you have any feedback.

*latest commit fixing coverage will be pushed shortly

tatiana

This looks great, @MrBones757. Thank you very much for improving this! It will be especially useful to K8s users.

I left minor comments in the code/docs.

It would also be great if you could update the correspondent docs, which previously stated this property was required:
https://github.com/astronomer/astronomer-cosmos/blob/main/docs/configuration/project-config.rst
https://astronomer.github.io/astronomer-cosmos/configuration/project-config.html

It would probably be safer if we could also add the behavior/a test to make sure that if the user does not specify dbt_project_path they are using LoadMethod.DBT_MANIFEST - otherwise, Cosmos should raise an exception.

cosmos/config.py

tests/test_config.py

…o properties. Fixed spelling

MrBones757 · 2023-10-12T02:27:24Z

Please let me know if there is anything else that needs doing on this MR!
look forward to the release of this so we can leverage this and other related changes!

tatiana

Thank you very much for working on this and addressing all the feedback, @MrBones757 !

@binhnq94

**Features** * Add support to model versioning available since dbt 1.6 by @binhnq94 in #516 * Add AWS Athena profile mapping by @benjamin-awd in #578 * Support customizing how dbt nodes are converted to Airflow by @tatiana in #503 * Make the arg ``dbt_project_path`` in the ``ProjectConfig`` optional by @MrBones757 in #581 **Bug fixes** * Fix Cosmos custom selector to support filtering a single model by @jlaneve and @harels in #576 * Fix using ``GoogleCloudServiceAccountDictProfileMapping`` together with ``LoadMethod.DBT_LS`` by @joppevos in #587 * Fix using the ``full_refresh`` argument in projects that contain tests by @EgorSemenov and @tatiana in #590 * Stop creating symbolic links for ``dbt_packages`` (solves ``LocalExecutor`` concurrency issue) by @tatiana in #600 **Others** * Docs: add reference to original Jaffle Shop project by @erdos2n in #583 * Docs: retries & note about DagBag error by @TJaniF in #592 * pre-commit updates in #575 and #585

tatiana · 2023-10-13T21:54:48Z

@MrBones757 This was released as part of 1.2:

tatiana · 2023-10-16T11:41:16Z

@MrBones757 we'll probably need further changes in coverter.py and perhaps dbt/graph.py so users can benefit from this feature from a DAG perspective.
ATM if a user does not give the dbt_project_path, they will probably face an exception similar to the one described in #601

…port dbt_project_path=None (#605) As part of the changes made #581, some downstream logic was missed relating to the handling of a None and String-based project dir. This MR attempts to remedy this issue by adding down steam support for the project dir being None (including generation of exceptions and guarding), as well as some property reference changes in the converter. Closes: #601 Co-authored-by: tabmra <[email protected]>

…port dbt_project_path=None (#605) As part of the changes made #581, some downstream logic was missed relating to the handling of a None and String-based project dir. This MR attempts to remedy this issue by adding down steam support for the project dir being None (including generation of exceptions and guarding), as well as some property reference changes in the converter. Closes: #601 Co-authored-by: tabmra <[email protected]> (cherry picked from commit 2f8d0e2)

make dbt_project_path optional, allow project_name to be user-defined

f653808

MrBones757 had a problem deploying to external October 9, 2023 08:55 — with GitHub Actions Error

🎨 [pre-commit.ci] Auto format from pre-commit.com hooks

1a51b5e

pre-commit-ci bot temporarily deployed to external October 9, 2023 08:57 Inactive

tatiana mentioned this pull request Oct 9, 2023

Make the arg dbt_project_path set in the ProjectConfig init optional #569

Closed

added additional test for coverage. Simplified Error Messages

9b53c53

MrBones757 temporarily deployed to external October 10, 2023 07:51 — with GitHub Actions Inactive

MrBones757 marked this pull request as ready for review October 10, 2023 07:55

MrBones757 requested a review from a team as a code owner October 10, 2023 07:55

MrBones757 requested a review from a team October 10, 2023 07:55

MrBones757 changed the title ~~WIP: Make the arg dbt_project_path in the ProjectConfig optional~~ Make the arg dbt_project_path in the ProjectConfig optional Oct 10, 2023

tatiana reviewed Oct 10, 2023

View reviewed changes

cosmos/config.py Outdated Show resolved Hide resolved

tests/test_config.py Outdated Show resolved Hide resolved

tatiana added this to the 1.3.0 milestone Oct 10, 2023

MrBones757 temporarily deployed to external October 10, 2023 09:38 — with GitHub Actions Inactive

Merge branch 'main' into optional-project-dir

4be04e0

tatiana temporarily deployed to external October 10, 2023 10:09 — with GitHub Actions Inactive

Updated test names, docstrings. Updated cosmos.config parsed fields t…

56aff39

…o properties. Fixed spelling

MrBones757 had a problem deploying to external October 11, 2023 04:08 — with GitHub Actions Error

Updated docs for project-config to reflect optional parameter

fc42acd

MrBones757 temporarily deployed to external October 11, 2023 05:12 — with GitHub Actions Inactive

Merge branch 'main' into optional-project-dir

cac70ca

tatiana temporarily deployed to external October 11, 2023 09:44 — with GitHub Actions Inactive

tatiana temporarily deployed to external October 11, 2023 10:26 — with GitHub Actions Inactive

tatiana approved these changes Oct 13, 2023

View reviewed changes

tatiana modified the milestones: 1.3.0, 1.2.0 Oct 13, 2023

Merge branch 'main' into optional-project-dir

d5c8449

tatiana temporarily deployed to external October 13, 2023 12:31 — with GitHub Actions Inactive

tatiana merged commit 5ae38f6 into astronomer:main Oct 13, 2023
40 checks passed

tatiana mentioned this pull request Oct 13, 2023

Release 1.2.0 #600

Merged

tatiana mentioned this pull request Oct 13, 2023

Fix converting dbt projects that have ProjectConfig.dbt_project_path set as strings #601

Closed

MrBones757 deleted the optional-project-dir branch October 17, 2023 00:20

MrBones757 mentioned this pull request Oct 17, 2023

Resolve errors occurring when dbt_project_path is str and partial support dbt_project_path=None #605

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make the arg dbt_project_path in the ProjectConfig optional #581

Make the arg dbt_project_path in the ProjectConfig optional #581

MrBones757 commented Oct 9, 2023

netlify bot commented Oct 9, 2023 •

edited

Loading

codecov bot commented Oct 9, 2023 •

edited

Loading

MrBones757 commented Oct 10, 2023 •

edited

Loading

tatiana left a comment •

edited

Loading

MrBones757 commented Oct 12, 2023 •

edited

Loading

tatiana left a comment

tatiana commented Oct 13, 2023

tatiana commented Oct 16, 2023

Make the arg dbt_project_path in the ProjectConfig optional #581

Make the arg dbt_project_path in the ProjectConfig optional #581

Conversation

MrBones757 commented Oct 9, 2023

Description

Related Issue(s)

Breaking Change?

Checklist

netlify bot commented Oct 9, 2023 • edited Loading

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

codecov bot commented Oct 9, 2023 • edited Loading

Codecov Report

MrBones757 commented Oct 10, 2023 • edited Loading

tatiana left a comment • edited Loading

Choose a reason for hiding this comment

MrBones757 commented Oct 12, 2023 • edited Loading

tatiana left a comment

Choose a reason for hiding this comment

tatiana commented Oct 13, 2023

tatiana commented Oct 16, 2023

netlify bot commented Oct 9, 2023 •

edited

Loading

codecov bot commented Oct 9, 2023 •

edited

Loading

MrBones757 commented Oct 10, 2023 •

edited

Loading

tatiana left a comment •

edited

Loading

MrBones757 commented Oct 12, 2023 •

edited

Loading