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

Added a warning when Python wheel wrapper needs to be used #807

Merged
merged 2 commits into from
Sep 27, 2023

Conversation

andrewnester
Copy link
Contributor

Changes

Added a warning when Python wheel wrapper needs to be used

Tests

Added unit tests + manual run with different bundle configurations

PythonWheelTask: &jobs.PythonWheelTask{},
JobClusterKey: "cluster1",
Libraries: []compute.Library{
{Whl: "/Workspace/Users/me@me.com/dist/test.whl"},
Copy link
Contributor

Choose a reason for hiding this comment

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

The test confirms that this should be fine and not trigger a warning.

Is this because it's an abs path and therefore we don't care about it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pietern yes, exactly, we wouldn't not wrap it anyway so it should be skipped

bundle/python/warning.go Outdated Show resolved Hide resolved
}

v := "v" + parts[0] + "." + parts[1]
return semver.Compare(v, "v13.1") < 0
Copy link
Contributor

Choose a reason for hiding this comment

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

This depends on the data security mode, no?

There's 13.1 for assigned clusters and 13.2 for shared clusters.

This is good enough for now, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pietern it indeed depends on it but the error received in this case is pretty clear by itself and actionable by users anyway

@andrewnester andrewnester added this pull request to the merge queue Sep 27, 2023
Merged via the queue into main with commit 3ee89c4 Sep 27, 2023
4 checks passed
@andrewnester andrewnester deleted the wheel-task-warning branch September 27, 2023 08:31
andrewnester added a commit that referenced this pull request Sep 27, 2023
Bundles:
 * Enable target overrides for pipeline clusters ([#792](#792)).
 * Add support for regex patterns in template schema ([#768](#768)).
 * Make the default `databricks bundle init` template more self-explanatory ([#796](#796)).
 * Make a notebook wrapper for Python wheel tasks optional ([#797](#797)).
 * Added a warning when Python wheel wrapper needs to be used ([#807](#807)).

Internal:
 * Added `process.Background()` and `process.Forwarded()` ([#804](#804)).

Dependency updates:
 * Bump golang.org/x/term from 0.11.0 to 0.12.0 ([#798](#798)).
 * Bump github.com/hashicorp/terraform-exec from 0.18.1 to 0.19.0 ([#801](#801)).
 * Bump golang.org/x/oauth2 from 0.11.0 to 0.12.0 ([#802](#802)).
@andrewnester andrewnester mentioned this pull request Sep 27, 2023
github-merge-queue bot pushed a commit that referenced this pull request Sep 27, 2023
Bundles:
* Enable target overrides for pipeline clusters
([#792](#792)).
* Add support for regex patterns in template schema
([#768](#768)).
* Make the default `databricks bundle init` template more
self-explanatory ([#796](#796)).
* Make a notebook wrapper for Python wheel tasks optional
([#797](#797)).
* Added a warning when Python wheel wrapper needs to be used
([#807](#807)).

Internal:
* Added `process.Background()` and `process.Forwarded()`
([#804](#804)).

Dependency updates:
* Bump golang.org/x/term from 0.11.0 to 0.12.0
([#798](#798)).
* Bump github.com/hashicorp/terraform-exec from 0.18.1 to 0.19.0
([#801](#801)).
* Bump golang.org/x/oauth2 from 0.11.0 to 0.12.0
([#802](#802)).
github-merge-queue bot pushed a commit that referenced this pull request Sep 29, 2023
…wheel tasks (#823)

## Changes
Follow up for #807 to also
validate configuration if existing cluster id is used.

## Tests
Added unit tests
hectorcast-db pushed a commit that referenced this pull request Oct 13, 2023
## Changes
Added a warning when Python wheel wrapper needs to be used

## Tests
Added unit tests + manual run with different bundle configurations
hectorcast-db pushed a commit that referenced this pull request Oct 13, 2023
Bundles:
* Enable target overrides for pipeline clusters
([#792](#792)).
* Add support for regex patterns in template schema
([#768](#768)).
* Make the default `databricks bundle init` template more
self-explanatory ([#796](#796)).
* Make a notebook wrapper for Python wheel tasks optional
([#797](#797)).
* Added a warning when Python wheel wrapper needs to be used
([#807](#807)).

Internal:
* Added `process.Background()` and `process.Forwarded()`
([#804](#804)).

Dependency updates:
* Bump golang.org/x/term from 0.11.0 to 0.12.0
([#798](#798)).
* Bump github.com/hashicorp/terraform-exec from 0.18.1 to 0.19.0
([#801](#801)).
* Bump golang.org/x/oauth2 from 0.11.0 to 0.12.0
([#802](#802)).
hectorcast-db pushed a commit that referenced this pull request Oct 13, 2023
…wheel tasks (#823)

## Changes
Follow up for #807 to also
validate configuration if existing cluster id is used.

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

Successfully merging this pull request may close these issues.

None yet

2 participants