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

Source-linked deployments for bundles in the workspace #1884

Merged
merged 37 commits into from
Nov 20, 2024

Conversation

ilyakuz-db
Copy link
Contributor

@ilyakuz-db ilyakuz-db commented Nov 5, 2024

Changes

This change adds a preset for source-linked deployments. It is enabled by default for targets in development mode if the Databricks CLI is running from the /Workspace directory on DBR. It does not have an effect when running the CLI anywhere else.

Key highlights:

  1. Files in this mode won't be uploaded to workspace
  2. Created resources will use references to source files instead of their workspace copies

Tests

  1. Apply preset unit test covering conditional logic
  2. High-level process target mode unit test for testing integration between mutators

@ilyakuz-db ilyakuz-db requested a review from pietern November 5, 2024 17:24
Copy link
Contributor

@shreyas-goenka shreyas-goenka left a comment

Choose a reason for hiding this comment

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

Thanks! Left a few comments for a few things to note when you do a proper pass!

bundle/config/mutator/apply_presets.go Outdated Show resolved Hide resolved
bundle/config/mutator/apply_presets.go Outdated Show resolved Hide resolved
bundle/config/mutator/apply_presets.go Outdated Show resolved Hide resolved
bundle/config/mutator/apply_presets.go Outdated Show resolved Hide resolved
bundle/config/mutator/apply_presets.go Outdated Show resolved Hide resolved
@ilyakuz-db ilyakuz-db changed the title WIP: Prototype for in-place deployments in DEV mode Prototype for in-place deployments in DEV mode Nov 14, 2024
@pietern pietern changed the title Source-linked deployments for bundles in the Workspace (In-Place) Source-linked deployments for bundles in the workspace Nov 19, 2024
if !isDatabricksWorkspace {
disabled := false
b.Config.Presets.SourceLinkedDeployment = &disabled
diags = diags.Extend(diag.Warningf("source-linked deployment is available only in the Databricks Workspace"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Warnings should include position/file/line information where possible.

This condition triggers only if the user explicitly configured the setting, so we should have this information. You can call b.Config.GetLocations() with the path to the setting to get these.

Not blocking for this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have this setting in targets.[target].presets.source_linked_deployment and this should be in the config indeed in that case, but we have nil in b.Config.Targets which makes location lookup not possible

It is assigned here

b.Config.Targets = nil

I can try to remove this nil assingment from there, not sure why it is needed. If I remove this line locations are available

Also we can show something like this without locations
image

Copy link
Contributor

Choose a reason for hiding this comment

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

You should just be able to use the locations from top-level presets.source_linked_deployment. That should include the target override locations as well IIRC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That works indeed, thanks!

image

Will make another PR later

bundle/config/presets.go Outdated Show resolved Hide resolved
@@ -23,6 +24,11 @@ func (m *upload) Name() string {
}

func (m *upload) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
if config.IsExplicitlyEnabled(b.Config.Presets.SourceLinkedDeployment) {
cmdio.LogString(ctx, "Source-linked deployment is enabled. Resources will point to the source files directly without making copies")
Copy link
Contributor

Choose a reason for hiding this comment

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

All users will see this, let's go through copy-editing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@@ -22,6 +24,9 @@ func WrapperWarning() bundle.Mutator {

func (m *wrapperWarning) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
if isPythonWheelWrapperOn(b) {
if config.IsExplicitlyEnabled(b.Config.Presets.SourceLinkedDeployment) {
cmdio.LogString(ctx, "Python wheel notebook wrapper is not available when using source-linked deployment mode. You can disable this mode by setting 'presets.source_linked_deployment: false'")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this a log vs a warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

ilyakuz-db and others added 2 commits November 20, 2024 11:33
Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Copy link

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 1884
  • Commit SHA: 66bf5f35c17f7e8652778b4d1975ea496239c933

Checks will be approved automatically on success.

@eng-dev-ecosystem-bot
Copy link
Collaborator

Test Details: go/deco-tests/11932942515

@pietern pietern merged commit 756e55f into main Nov 20, 2024
9 checks passed
@pietern pietern deleted the tmp/in-place-prototype branch November 20, 2024 12:22
pietern added a commit that referenced this pull request Nov 20, 2024
**Note:** the `bundle generate` command now uses the `.<resource-type>.yml`
sub-extension for the configuration files it writes. Existing configuration
files that do not use this sub-extension are renamed to include it.

Bundles:
 * Make `TableName` field part of quality monitor schema ([#1903](#1903)).
 * Do not prepend paths starting with ~ or variable reference ([#1905](#1905)).
 * Fix workspace extensions filer accidentally reading notebooks ([#1891](#1891)).
 * Fix template initialization when running on Databricks ([#1912](#1912)).
 * Source-linked deployments for bundles in the workspace ([#1884](#1884)).
 * Added integration test to deploy bundle to /Shared root path ([#1914](#1914)).
 * Update filenames used by bundle generate to use `.<resource-type>.yml` ([#1901](#1901)).

Internal:
 * Extract functionality to detect if the CLI is running on DBR ([#1889](#1889)).
 * Consolidate test helpers for `io/fs` ([#1906](#1906)).
 * Use `fs.FS` interface to read template ([#1910](#1910)).
 * Use `filer.Filer` to write template instantiation ([#1911](#1911)).
github-merge-queue bot pushed a commit that referenced this pull request Nov 20, 2024
**Note:** the `bundle generate` command now uses the
`.<resource-type>.yml`
sub-extension for the configuration files it writes. Existing
configuration
files that do not use this sub-extension are renamed to include it.

Bundles:
* Make `TableName` field part of quality monitor schema
([#1903](#1903)).
* Do not prepend paths starting with ~ or variable reference
([#1905](#1905)).
* Fix workspace extensions filer accidentally reading notebooks
([#1891](#1891)).
* Fix template initialization when running on Databricks
([#1912](#1912)).
* Source-linked deployments for bundles in the workspace
([#1884](#1884)).
* Added integration test to deploy bundle to /Shared root path
([#1914](#1914)).
* Update filenames used by bundle generate to use `.<resource-type>.yml`
([#1901](#1901)).

Internal:
* Extract functionality to detect if the CLI is running on DBR
([#1889](#1889)).
* Consolidate test helpers for `io/fs`
([#1906](#1906)).
* Use `fs.FS` interface to read template
([#1910](#1910)).
* Use `filer.Filer` to write template instantiation
([#1911](#1911)).
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.

5 participants