When globbing for wheels, only consider most recent version; do not clean up dist/build/__pycache__ by denik · Pull Request #2982 · databricks/cli

denik · 2025-06-03T16:07:00Z

Changes

When glob like *.whl is expanded, if there are multiple versions of the same package, we will only consider the most recent version.
Do not forcibly delete dist, build and __pycache__ folders on every deploy.

Why

Deleting dist folder is surprising and invasive (e.g. see #2969).

In order to avoid doing that but keep old "dist/*.whl" globs working, we're modifying glob behaviour to do additional filtering so that if there are more than 1 wheel from the same package, we'll only upload the one with most recent version.

Tests

New acceptance test simulates situation in #2969

Existing acceptance test show that wheels that were uploaded twice (from .internal and dist folders) are not uploaded once, which is more inline with the expectations.

pietern

Looks good. The sync exclude can be omitted, IMO.

pietern · 2025-06-03T17:57:58Z

acceptance/bundle/artifacts/whl_change_version/output.txt

+=== Expecting 1 wheel to be uploaded
+>>> jq .path
+"/api/2.0/workspace-files/import-file/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal/my_test_code-0.1.0-py3-none-any.whl"
+"/api/2.0/workspace-files/import-file/Workspace/Users/[USERNAME]/.bundle/test-bundle/default/artifacts/.internal/my_test_code-0.2.0-py3-none-any.whl"


Why does it upload both?

https://github.com/databricks/cli/pull/2982/files#r2128394135

pietern · 2025-06-03T17:58:17Z

acceptance/bundle/artifacts/whl_change_version/output.txt

+dist/my_test_code-0.2.0-py3-none-any.whl
+
+=== Expecting 1 wheel in libraries section in /jobs/create
+>>> jq -s .[] | select(.path=="/api/2.2/jobs/create") | .body.tasks out.requests.txt


This will call the update API.

Updated, thanks.

pietern · 2025-06-03T17:58:35Z

acceptance/bundle/artifacts/whl_change_version/output.txt

+>>> [CLI] bundle deploy
+Building python_artifact...
+Uploading dist/my_test_code-0.1.0-py3-none-any.whl...
+Uploading dist/my_test_code-0.2.0-py3-none-any.whl...


Why are both uploaded if the glob only matches the 0.1.0 one?

The glob in libraries matches 0.1.0 but the glob in artifacts section matches 0.2.0.

Those filters operate independently from different mutators so each makes their own decision.

Perhaps it's a sign that glob expansion should be done from a single place with a global tracker of wheel versions.

Is the glob in artifacts implicit or a default? If so, that seems surprising.

Is the glob in artifacts implicit or a default?

Not sure what you mean? implicit == default, no?

If so, that seems surprising.

Somewhat. It would also be surprising if default *.whl behaved differently from explicit *.whl

pietern · 2025-06-03T18:01:32Z

bundle/deploy/files/sync.go


+	// We used to delete __pycache__ and build and most of the dist, so now we're excluding it manually
+	// TODO: if users include those manually, then we should not exclude it?
+	excludes := append(b.Config.Sync.Exclude, "__pycache__", "build", "dist")


This can be a regression. Most users will exclude dist in their .gitignore anyway. If they have an artifact build and include it in their sync configuration, then it would have been synchronized just fine previously.

By not deleting it, we can potentially synchronize more than we need, but still should observe the .gitignore rules and let it happen, so that the remote tree matches what you have locally.

My concern was that we might start uploading a bunch of stuff for some users that we did not do before.

However, there is self-serving fix (update .gitignore or sync.exclude) so it's not a big deal.

It's hard to come up with sync patterns that match old behaviour exactly, so I removed this.

pietern · 2025-06-03T18:23:07Z

libs/patchwheel/filter.go

@@ -0,0 +1,57 @@
+package patchwheel


Feels like this belongs in libs/python more than patchwheel. WDYT?

libs/python is about finding python interpreter.

libs/patchwheel is about patching wheel but also parsing wheel names. It's a better fit but perhaps the name should be changed to something like wheel.

I'd also say that it better belongs to libs/python or renamed to libs/wheel

pietern · 2025-06-03T18:25:20Z

acceptance/bundle/artifacts/whl_change_version/script

+print_status
+
+title "Update glob to target 0.1.0 version\n"
+update_file.py databricks.yml './dist/*.whl' './dist/my*0.1.0*.whl'


With this new glob it doesn't exercise the latest-version-matching code.

Yes, this tests that old wheel is still present, as in #2969

Still, good point, I changed the test to exercise new code as well.

andrewnester · 2025-06-04T08:10:10Z

acceptance/bundle/artifacts/whl_change_version/output.txt

+dist/my_test_code-0.1.0-py3-none-any.whl
+
+=== Expecting 1 wheel in libraries section in /jobs/create
+>>> jq -s .[] | select(.path=="/api/2.2/jobs/create") | .body.tasks out.requests.txt


It seems like nothing is matched here, is it expected?

fixed to look for /reset instead.

## Changes - Insert regular glob into default settings instead of custom function for finding wheels. - Remove code to explicitly fail the build if wheels are not found. ## Why - Helps in #2982 where we modify glob expansion to filter out same-name wheels. - Failed build will stop deploy regardless, because there is return code check and glob expansion check, no need for custom code there. - Simpler & more consistent behaviour: whl behave like those without 'type: whl'. ## Tests Existing tests. --------- Co-authored-by: Pieter Noordhuis <[email protected]>

andrewnester · 2025-06-06T10:18:45Z

libs/patchwheel/filter.go

@@ -0,0 +1,57 @@
+package patchwheel


I'd also say that it better belongs to libs/python or renamed to libs/wheel

pietern · 2025-06-11T04:37:43Z

acceptance/bundle/artifacts/whl_change_version/output.txt

+>>> [CLI] bundle deploy
+Building python_artifact...
+Uploading dist/my_test_code-0.1.0-py3-none-any.whl...
+Uploading dist/my_test_code-0.2.0-py3-none-any.whl...


Is the glob in artifacts implicit or a default? If so, that seems surprising.

pietern · 2025-06-11T04:39:08Z

libs/patchwheel/filter_test.go

+		"other-0.9.0-py3-none-any.whl",
+		"project_name_bvs7tide6bhhpjy4dmcsb2qg44-0.0.1+20250604.74804-py3-none-any.whl",
+		"not-a-wheel.whl",
+


cleaned up, thanks.

…ously deleted stuff when syncing

## Release v0.256.0 ### Bundles * When building Python artifacts as part of "bundle deploy" we no longer delete `dist`, `build`, `*egg-info` and `__pycache__` directories ([#2982](#2982)) * When glob for wheels is used, like "\*.whl", it will filter out different version of the same package and will only take the most recent version ([#2982](#2982)) * Add preset `presets.artifacts_dynamic_version` that automatically enables `dynamic_version: true` on all "whl" artifacts ([#3074](#3074)) * Update client version to "2" for the serverless variation of the default-python template ([#3083](#3083)) * Fix reading dashboard contents when the sync root is different than the bundle root ([#3006](#3006)) * Fix variable resolution for lookup variables with other references ([#3054](#3054)) * Allow users to override the Terraform version to use by setting the `DATABRICKS_TF_VERSION` environment variable ([#3069](#3069))

denik requested review from andrewnester, anton-107, pietern and shreyas-goenka as code owners June 3, 2025 16:07

denik temporarily deployed to test-trigger-is June 3, 2025 16:07 — with GitHub Actions Inactive

denik temporarily deployed to test-trigger-is June 3, 2025 16:15 — with GitHub Actions Inactive

denik temporarily deployed to test-trigger-is June 3, 2025 16:19 — with GitHub Actions Inactive

denik force-pushed the denik/whl-smart-glob branch from 5dfdb25 to 45dc590 Compare June 3, 2025 16:24

denik temporarily deployed to test-trigger-is June 3, 2025 16:25 — with GitHub Actions Inactive

pietern reviewed Jun 3, 2025

View reviewed changes

andrewnester reviewed Jun 4, 2025

View reviewed changes

This was referenced Jun 4, 2025

When collecting libraries, convert them to absolute path #2986

Closed

Simplify & fix default configuration for whl artifacts #2987

Merged

denik marked this pull request as draft June 4, 2025 09:40

denik force-pushed the denik/whl-smart-glob branch from 45dc590 to b009173 Compare June 5, 2025 08:46

denik temporarily deployed to test-trigger-is June 5, 2025 08:46 — with GitHub Actions Inactive

denik temporarily deployed to test-trigger-is June 5, 2025 09:25 — with GitHub Actions Inactive

denik temporarily deployed to test-trigger-is June 5, 2025 09:44 — with GitHub Actions Inactive

denik temporarily deployed to test-trigger-is June 5, 2025 09:50 — with GitHub Actions Inactive

denik marked this pull request as ready for review June 5, 2025 09:51

denik requested review from andrewnester and pietern June 5, 2025 09:51

denik temporarily deployed to test-trigger-is June 5, 2025 10:02 — with GitHub Actions Inactive

denik temporarily deployed to test-trigger-is June 5, 2025 10:03 — with GitHub Actions Inactive

denik temporarily deployed to test-trigger-is June 5, 2025 10:07 — with GitHub Actions Inactive

denik temporarily deployed to test-trigger-is June 5, 2025 12:56 — with GitHub Actions Inactive

andrewnester approved these changes Jun 6, 2025

View reviewed changes

pietern approved these changes Jun 11, 2025

View reviewed changes

when collecting libraries, convert them to absolute path

87cae7f

denik added 15 commits June 12, 2025 11:26

only join is not abs already

d6b012a

add filter + version

138190a

enable filtering; disable pythin clean ups add acc test; ignore previ…

6f48b6b

…ously deleted stuff when syncing

no Background

e35e713

clean up

e7f1f41

update test

21370e4

clean up and extend tests

980a066

fix

2e4445e

clean up & rebase

8b24696

rewrite test

a7f36bf

stricter defaultExcludes

2cc0910

remove default sync excludes

fd214ff

revert convertion to abs in upload.go

670bd5d

update NEXT_CHANGELOG

b7518f6

formatting

c0d9c66

denik force-pushed the denik/whl-smart-glob branch from 0d60bc5 to c0d9c66 Compare June 12, 2025 09:28

denik temporarily deployed to test-trigger-is June 12, 2025 09:28 — with GitHub Actions Inactive

fix formatting

4f46907

denik temporarily deployed to test-trigger-is June 12, 2025 09:31 — with GitHub Actions Inactive

clean up newline

00b560a

denik temporarily deployed to test-trigger-is June 12, 2025 10:04 — with GitHub Actions Inactive

denik enabled auto-merge June 12, 2025 10:04

denik added this pull request to the merge queue Jun 12, 2025

Merged via the queue into main with commit 8ffff79 Jun 12, 2025
10 checks passed

denik deleted the denik/whl-smart-glob branch June 12, 2025 10:51

andrewnester mentioned this pull request Jun 23, 2025

DABs cleanup of dist and .internal causing locked wheel versions to fail #2969

Closed

Conversation

denik commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Why

Tests

Uh oh!

pietern left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

denik commented Jun 3, 2025 •

edited

Loading