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.

Sign up for GitHub

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

Jump to bottom

Skip mocking of keyfile_json field #587

Merged
merged 4 commits into from
Oct 10, 2023
Merged

Skip mocking of keyfile_json field #587

merged 4 commits into from
Oct 10, 2023

Conversation

joppevos
Copy link
Contributor

@joppevos joppevos commented Oct 10, 2023 •
edited
Loading

Description

Makes the required field keyfile_json empty, so that dbt can parse it if the project.yaml file is being mocked.

Related Issue(s)

closes #586

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

@joppevos joppevos requested a review from a team as a code owner October 10, 2023 01:20
@joppevos joppevos requested a review from a team October 10, 2023 01:20
@netlify
Copy link

netlify bot commented Oct 10, 2023 •
edited
Loading

👷 Deploy Preview for amazing-pothos-a3bca0 processing.

Name Link
🔨 Latest commit 05ad59a
🔍 Latest deploy log https://app.netlify.com/sites/amazing-pothos-a3bca0/deploys/65252299a13a07000875a872

@pre-commit-ci pre-commit-ci bot temporarily deployed to external October 10, 2023 01:23 Inactive
tatiana
tatiana reviewed Oct 10, 2023
View reviewed changes
Copy link
Collaborator

@tatiana tatiana left a comment

Choose a reason for hiding this comment

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

Hi @joppevos , thanks a lot for the fix! Please add a test to ensure we don't have any regressions moving forward

@tatiana tatiana added this to the 1.3.0 milestone Oct 10, 2023
@codecov
Copy link

codecov bot commented Oct 10, 2023 •
edited
Loading

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (a073ecc) 92.80% compared to head (05ad59a) 93.01%.
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #587      +/-   ##
==========================================
+ Coverage   92.80%   93.01%   +0.20%     
==========================================
  Files          49       51       +2     
  Lines        2002     2019      +17     
==========================================
+ Hits         1858     1878      +20     
+ Misses        144      141       -3
Files Coverage Δ
cosmos/profiles/__init__.py 96.29% <100.00%> (+0.14%) ⬆️
cosmos/profiles/athena/__init__.py 100.00% <100.00%> (ø)
cosmos/profiles/athena/access_key.py 100.00% <100.00%> (ø)
.../profiles/bigquery/service_account_keyfile_dict.py 100.00% <100.00%> (+5.40%) ⬆️

... and 1 file with indirect coverage changes

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

@joppevos joppevos temporarily deployed to external October 10, 2023 09:45 — with GitHub Actions Inactive
@joppevos
Copy link
Contributor Author

joppevos commented Oct 10, 2023 •
edited
Loading

Hi @joppevos , thanks a lot for the fix! Please add a test to ensure we don't have any regressions moving forward

Thanks @tatiana , I added a simple test. We could also run a dbt ls test on the mocked profile, but then we are kinda testing dbt itself for not breaking over time.

Would be great to get this solved quickly, cause you can't get a DAG pass general Airflow CI tests when it's failing on parsing.

@tatiana
Copy link
Collaborator

tatiana commented Oct 10, 2023

Hi @joppevos , thanks a lot for the fix! Please add a test to ensure we don't have any regressions moving forward

Thanks @tatiana , I added a simple test. We could also run a dbt ls test on the mocked profile, but then we are kinda testing dbt itself for not breaking over time.

Would be great to get this solved quickly, cause you can't get a DAG pass general Airflow CI tests when it's failing on parsing.

Thanks, @joppevos , this is very helpful.
This will be released as part of the 1.2 release, this week.

@tatiana tatiana modified the milestones: 1.3.0, 1.2.0 Oct 10, 2023
@tatiana tatiana temporarily deployed to external October 10, 2023 10:08 — with GitHub Actions Inactive
tatiana
tatiana approved these changes Oct 10, 2023
View reviewed changes
Copy link
Collaborator

@tatiana tatiana left a comment

Choose a reason for hiding this comment

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

Excellent and quick fix, thanks, @joppevos!

@tatiana tatiana merged commit 9460312 into astronomer:main Oct 10, 2023
39 checks passed
@joppevos joppevos deleted the skip-mock-of-keyfile branch October 11, 2023 01:21
@tatiana tatiana mentioned this pull request Oct 13, 2023
Merged
tatiana added a commit that referenced this pull request Oct 13, 2023
@tatiana
Release 1.2.0 (#600)
d61f120 
**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
Copy link
Collaborator

tatiana commented Oct 13, 2023

This was released as part of 1.2:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tatiana tatiana tatiana approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.

Mock values unable to be parsed by dbt
2 participants
@joppevos @tatiana