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

Add experimental cache_selected_only config #5036

Merged
merged 9 commits into from
Apr 12, 2022
Merged

Conversation

jtcohen6
Copy link
Contributor

resolves #4688
rebased version of #4860
more tracking + testing to come in #4961

Only other changes:

  • --cache-selected-only (kebab case) instead of --cached_selected_only (snake case)
  • Add a breaking change note for the internal adapter method (Python code) with a change in its signature. Among our plugins, only dbt-postgres reimplements this method; all the others inherit the base version.

I'd love to include this as "experimental" functionality in v1.1 if possible. We would document it as such: dbt-labs/docs.getdbt.com#1327

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have added information about my change to be included in the CHANGELOG.

@jtcohen6 jtcohen6 requested a review from a team as a code owner April 12, 2022 13:30
@jtcohen6 jtcohen6 requested a review from a team April 12, 2022 13:30
@jtcohen6 jtcohen6 requested review from a team as code owners April 12, 2022 13:30
@cla-bot cla-bot bot added the cla:yes label Apr 12, 2022
@jtcohen6 jtcohen6 changed the title Jerco/cherry pick 4860 Add experimental cache_selected_only config Apr 12, 2022
Copy link
Contributor

@gshank gshank left a comment

Choose a reason for hiding this comment

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

Looks good! I see there's only a test for the flag setting not the caching though.

@jtcohen6
Copy link
Contributor Author

@gshank You're right :) This is definitely going to be "experimental" / "use at your own risk," but worth at least one test. I did a bad thing, and just extended an old test for caching, rather than converting.

@gshank
Copy link
Contributor

gshank commented Apr 12, 2022

Changing an existing test is perfectly legit :) They ought to behave the same...

@jtcohen6 jtcohen6 merged commit bacc891 into main Apr 12, 2022
@jtcohen6 jtcohen6 deleted the jerco/cherry-pick-4860 branch April 12, 2022 16:04
@jtcohen6 jtcohen6 mentioned this pull request Apr 12, 2022
4 tasks
Copy link
Contributor

@ChenyuLInx ChenyuLInx left a comment

Choose a reason for hiding this comment

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

Late to the party but this looks great!

VersusFacit pushed a commit that referenced this pull request Apr 14, 2022
* cache schema for selected models

* Create Features-20220316-003847.yaml

* rename flag, update postgres adapter

rename flag to cache_selected_only, update postgres adapter: function _relations_cache_for_schemas

* Update Features-20220316-003847.yaml

* added test for cache_selected_only flag

* formatted as per pre-commit

* Add breaking change note for adapter plugin maintainers

* Fix whitespace

* Add a test

Co-authored-by: karunpoudel-chr <poudel.karun@gmail.com>
Co-authored-by: karunpoudel-chr <62040859+karunpoudel@users.noreply.github.com>
agoblet pushed a commit to BigDataRepublic/dbt-core that referenced this pull request May 20, 2022
* cache schema for selected models

* Create Features-20220316-003847.yaml

* rename flag, update postgres adapter

rename flag to cache_selected_only, update postgres adapter: function _relations_cache_for_schemas

* Update Features-20220316-003847.yaml

* added test for cache_selected_only flag

* formatted as per pre-commit

* Add breaking change note for adapter plugin maintainers

* Fix whitespace

* Add a test

Co-authored-by: karunpoudel-chr <poudel.karun@gmail.com>
Co-authored-by: karunpoudel-chr <62040859+karunpoudel@users.noreply.github.com>
@rmargarint-nydig
Copy link

I realized this is a comment on an old PR, but I was experimenting today with cache_selected_only to speed up start up time on our test execution pipelines which run as part of Airflow orchestration. It seems that cache_selected_only only seems to affect dbt run but not dbt test --select "test_name"? From what I've traced, it seems get_model_schemas(...) will return an empty set when one of the selected uids is a test resource, which will result in the entire set of schemas being pulled into the cache. Is this expected?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-168] Cache objects for selected resources only?
5 participants