-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fixups for deps lock file #9147
Conversation
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
1 similar comment
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #9147 +/- ##
==========================================
- Coverage 86.66% 86.60% -0.07%
==========================================
Files 179 179
Lines 26569 26598 +29
==========================================
+ Hits 23027 23036 +9
- Misses 3542 3562 +20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!!!
) | ||
|
||
@mock.patch("dbt.deps.tarball.get_downloads_path") | ||
def test_tarball_pinned_package_contract_with_unrendered(self, mock_get_downloads_path): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice unittest!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(ty michelle!)
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.7.latest 1.7.latest
# Navigate to the new working tree
cd .worktrees/backport-1.7.latest
# Create a new branch
git switch --create backport-9147-to-1.7.latest
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 32fde755043134dd8a2f5835db9c50e2f4383cb8
# Push it to GitHub
git push --set-upstream origin backport-9147-to-1.7.latest
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.7.latest Then, create a pull request where the |
* Fixups for deps lock file (#9147) * Update git revision with commit SHA * Use PackageRenderer for lock file * add unit tests for git and tarball packages * deepcopy unrendered_packages_data before iteration, fix remaining tests * Add functional tests * Add changelog entries * Assert one more --------- Co-authored-by: Michelle Ark <[email protected]> * Restore warning on unpinned git packages --------- Co-authored-by: Michelle Ark <[email protected]>
resolves #9050
resolves #9127
Problem
git
packages to a specific commit SHA, before writing topackage-lock.json
. Then we wouldn't warn that the package is "unpinned" in subsequent invocation ([CT-3368] [Regression]warn-unpinned
is ignored (dbt deps, packages.yml) #9050)package-lock.json
([CT-3411] [Regression] env_var file enforcement not correct for projects with subpackages that contain env_vars #9127)Solution
git
packagerevision
with actual git commit SHAPackageRenderer
for rereadingpackage-lock.json
, plus plumbing & tests to support.During live pairing with @ChenyuLInx @MichelleArk last Wednesday, we added some defensive code (to support backporting) and an extra logging event (as a treat).
Checklist