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

Deps lock by justbldwn #8408

Merged
merged 31 commits into from
Oct 10, 2023
Merged

Deps lock by justbldwn #8408

merged 31 commits into from
Oct 10, 2023

Conversation

ChenyuLInx
Copy link
Contributor

@ChenyuLInx ChenyuLInx commented Aug 15, 2023

resolves #6643
This is work from @justbldwn. original PR at #6735.
We(core team) are working on getting it up-to-date with the changes made recently about dependencies.yml and updating some small things to get it into ready to merge state.

We would like this to be a non breaking change, which means dbt deps will behave the same(-ish, caveat below). With this in mind, considering the constrain of how click commands are structured, also talked to Jerco about the philosophy of CLI commands of dbt-core, we are updating the dbt deps add and dbt deps lock to just dbt deps --add and dbt deps --lock. @justbldwn let us know if you feel strongly about it should be sub command vs a flag.

The caveat with the current implementation:
Previously, running dbt deps will upgrade packages is version is specified as range and new version come out. For example, local package A is version 0.1.1, specified version [> 0.1.0, < 0.2.0]. Latest version is 0.1.5, running deps previously will install 0.1.5, but now with the lock file approach it will actually still install 0.1.1. @dbeatty10 how you feel about it?

Description of behavior

  • running dbt deps first time will generate package-lock.yml that contains all resolved packages
  • running dbt deps with package spec in depenedencies.yml or packages.yml not updated, dbt-core will just install from package-lock.yml.(hence the caveat above)
  • update package spec and run deps again, both package lock and package get updated accordingly
  • There's the --add flag, when specified, along with --package and --version, dbt-core will try to update the package spec and run deps depending on whether --dry-run is specified. The original PR provide better examples. Remember --dry-run is not a flag and you do not need to say --dry-run True, just --dry-run.
  • There's also the flag --update which can be used when you have packaged specified in version range and want to try to update to the new package available. Previous Deps behavior is actually dbt deps --update after this change.
  • There's another flag --lock which will generate the new lock file but not actually install any packages.

TODO for this PR

  • Add logic to store the hash of the previous defined packages there to automatically run deps when content packages got updated.

  • Add a --upgrade flag to upgrade the locks file if user specified version bound and there's new versions released

  • Add a --lock flag to only generate a new lock file without actually install

  • make sure --add works with dependencies.yml

  • reorganize lock and add as args so deps will not be a breaking change, and we keep the consistent way of CLI commands where the commands are mostly verbs, and we alter the behavior of command with flags.

  • reformat the saved yml file

  • confirm how many times we will have to pull down a git repo when running deps with packages-lock not exist, and determine optimizing/not (Behavior here is the same as before, it will pull down git packages during resolution).

  • fix unittest

TODO for opening follow up PR

There's a set of optimizations mentioned in #6643 that is enabled by the new lock yml. We should open follow up PRs about it

  • install only diff package
  • run deps before every command

Checklist

  • I have read the contributing guide and understand what's expected of me
  • 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
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX

@ChenyuLInx ChenyuLInx requested review from a team as code owners August 15, 2023 07:16
@cla-bot cla-bot bot added the cla:yes label Aug 15, 2023
@ChenyuLInx ChenyuLInx removed request for a team, gshank and nathaniel-may August 15, 2023 07:16
@ChenyuLInx ChenyuLInx marked this pull request as draft August 15, 2023 07:16
@ChenyuLInx ChenyuLInx self-assigned this Aug 15, 2023
@ChenyuLInx ChenyuLInx added the user docs [docs.getdbt.com] Needs better documentation label Aug 15, 2023
@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (70b2e15) 86.54% compared to head (5676771) 86.43%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8408      +/-   ##
==========================================
- Coverage   86.54%   86.43%   -0.11%     
==========================================
  Files         176      176              
  Lines       25856    26009     +153     
==========================================
+ Hits        22376    22481     +105     
- Misses       3480     3528      +48     
Flag Coverage Δ
integration 83.23% <93.33%> (-0.06%) ⬇️
unit 64.86% <38.66%> (-0.21%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
core/dbt/cli/params.py 100.00% <100.00%> (ø)
core/dbt/config/project.py 97.39% <100.00%> (+<0.01%) ⬆️
core/dbt/constants.py 100.00% <100.00%> (ø)
core/dbt/deps/resolver.py 88.42% <100.00%> (+0.92%) ⬆️
core/dbt/events/types.py 98.66% <100.00%> (+0.01%) ⬆️
core/dbt/events/types_pb2.py 1.40% <ø> (+0.07%) ⬆️
core/dbt/cli/main.py 98.43% <83.33%> (-0.30%) ⬇️
core/dbt/cli/option_types.py 79.06% <70.00%> (-2.75%) ⬇️
core/dbt/task/deps.py 94.32% <94.25%> (-0.68%) ⬇️

... and 6 files with indirect coverage changes

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

core/dbt/cli/params.py Outdated Show resolved Hide resolved
core/dbt/cli/params.py Outdated Show resolved Hide resolved
core/dbt/task/deps.py Outdated Show resolved Hide resolved
core/dbt/task/deps.py Outdated Show resolved Hide resolved
core/dbt/cli/params.py Outdated Show resolved Hide resolved
core/dbt/task/deps.py Outdated Show resolved Hide resolved
@ChenyuLInx ChenyuLInx merged commit 549dbf3 into main Oct 10, 2023
49 of 50 checks passed
@ChenyuLInx ChenyuLInx deleted the deps_lock_by_justbldwn branch October 10, 2023 04:05
@FishtownBuildBot
Copy link
Collaborator

Opened a new issue in dbt-labs/docs.getdbt.com: dbt-labs/docs.getdbt.com#4235

dbeatty10 added a commit to dbt-labs/docs.getdbt.com that referenced this pull request Nov 15, 2023
[Preview](https://docs-getdbt-com-git-dbeatty-fix-dbt-deps-add-package-dbt-labs.vercel.app/reference/commands/deps)

## What are you changing in this pull request and why?

Fixing the code examples per
dbt-labs/dbt-core#9076 by using test examples
from dbt-labs/dbt-core#8408

## Checklist
- [x] Review the [Content style
guide](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/content-style-guide.md)
and [About
versioning](https://github.com/dbt-labs/docs.getdbt.com/blob/current/contributing/single-sourcing-content.md#adding-a-new-version)
so my content adheres to these guidelines.
- [x] I've checked that the code examples work
- [x] I've confirmed that the preview renders correctly
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:yes user docs [docs.getdbt.com] Needs better documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-1845] [Feature] Write a file of installed versions during dbt deps
5 participants