Add development runs #522

lennartkats-db · 2023-06-23T11:00:20Z

(This is a new copy of #493, as my fork was broken when this repository was made public.)

This implements the "debug run" or "development run" functionality that we desire for DABs in the workspace / IDE.

bundle.yml changes

In bundle.yml, there should be a "dev" environment that is marked as mode: debug:

environments:
  dev:
    default: true
    mode: development # future accepted values might include pull_request, production

Setting mode to development indicates that this environment is used just for running things for development. This results in several changes to deployed assets:

All assets will get '[dev]' in their name and will get a 'dev' tag
All assets will be hidden from the list of assets (future work; e.g. for jobs we would have a special job_type that hides it from the list)
All deployed assets will be ephemeral (future work, we need some form of garbage collection)
Pipelines will be marked as 'development: true'
Jobs can run on development compute through the --compute-id parameter in the CLI
Jobs get their schedule / triggers paused
Jobs get concurrent runs (it's really annoying if your runs get skipped because the last run was still in progress)

Other accepted values for mode are default (which does nothing) and pull-request (which is reserved for future use).

CLI changes

To run a single job called "shark_sighting" on existing compute, use the following commands:

$ databricks bundle deploy --compute-id 0617-201942-9yd9g8ix
$ databricks bundle run shark_sighting

which would deploy and run a job called "[dev] shark_sightings" on the compute provided. Note that --compute-id is not accepted in production environments, so we show an error if mode: development is not used.

The run --deploy command offers a convenient shorthand for the common combination of deploying & running:

$ export DATABRICKS_COMPUTE=0617-201942-9yd9g8ix
$ bundle run --deploy shark_sightings

The --deploy addition isn't really essential and I welcome feedback 🤔 I played with the idea of a "debug" or "dev" command but that seemed to only make the option space even broader for users. The above could work well with an IDE or workspace that automatically sets the target compute.

One more thing I added isrun --no-wait can now be used to run something without waiting for it to be completed (useful for IDE-like environments that can display progress themselves).

$ bundle run --deploy shark_sightings --no-wait

- Add "mode: debug" property for environments - Add "--deploy", "--compute", "--no-wait" CLI flags

cmd/bundle/run.go

lennartkats-db · 2023-06-23T11:01:29Z

cmd/bundle/deploy.go

+	b.Config.Bundle.Lock.Force = forceDeploy
+
+	if computeID == "" {
+		computeID = os.Getenv("DATABRICKS_CLUSTER_ID")


I changed this to DATABRICKS_CLUSTER_ID for now since we have that precedent. Likely we need to change both of them over time.

The field should also be called cluster_id if we pull the cluster ID. We can have one for each of clusters, compute, warehouses. The flag parsing + environment overrides should happen centrally and not by reusing globals from other commands.

compute and compute_id are the general terms we're using for serverless jobs / interactive to indicate both clusters and "serverless" clusters. Warehouses are separate. I'd rather not change the name to cluster_id if it's really an id that also works for serverless compute.

OK, computeID works. Can you remove the getenv check though? It will mean that deployment fails if that value is set if it's not in dev mode, which would be unexpected behavior to me. Having the flag alone captures the intent without that failure mode.

I updated it so we only fail if the override is set in the bundle or CLI, and not when it's set in the environment.

bundle/phases/initialize.go

pietern · 2023-07-03T11:54:28Z

cmd/bundle/deploy.go

+	b.Config.Bundle.Lock.Force = forceDeploy
+
+	if computeID == "" {
+		computeID = os.Getenv("DATABRICKS_CLUSTER_ID")


The field should also be called cluster_id if we pull the cluster ID. We can have one for each of clusters, compute, warehouses. The flag parsing + environment overrides should happen centrally and not by reusing globals from other commands.

cmd/bundle/run.go

bundle/config/bundle.go

pietern · 2023-07-05T19:38:11Z

bundle/config/bundle.go

+	Mode Mode `json:"mode,omitempty"`
+
+	// Overrides the compute used for jobs and other supported assets.
+	Compute string `json:"override_compute,omitempty" bundle:"readonly"`


Why use a different name for the JSON field and the Go field?

Does the compute field in the jobs API apply to clusters as well? Or do you take this value and plan to use it in both the compute and cluster fields in the job specs?

Also, it is used as ID, so should be called ComputeID to match.

What is the expected behavior if this value is set by the user?

I updated this to just say computeId/ComputeID. Is "-id" the customer-facing convention we have throughout the spec?

Note that this wasn't meant to set from the config, hence the "readonly," but that doesn't seem to do what I expected it to. I updated it so it can be set at the environment level now.x

In terms of naming, what I can do is also change the parameter name to say "--compute-id"/"-c" instead of "--compute"? It's a bit longer but I tentatively made that change.

pietern · 2023-07-05T19:39:50Z

bundle/config/mutator/populate_current_user.go

@@ -18,6 +18,10 @@ func (m *populateCurrentUser) Name() string {
 }

 func (m *populateCurrentUser) Apply(ctx context.Context, b *bundle.Bundle) error {
+	if b.Config.Workspace.CurrentUser != nil {
+		return nil
+	}


Curious: has this run twice in your testing?

Yes, it would, but I think that may have only happened with databricks deploy --run. So I'll remove this.

We should actually have a general fix to make sure w.CurrentUser.Me() is cached, since it is very very slow and was still being called a second time in some other cases. This could be part of a general initiative to make things a bit faster.

bundle/config/mutator/process_environment_mode.go

pietern · 2023-07-05T19:41:16Z

bundle/config/mutator/process_environment_mode.go

+			r.Jobs[i].MaxConcurrentRuns = debugConcurrentRuns
+		}
+		if r.Jobs[i].Schedule != nil {
+			r.Jobs[i].Schedule.PauseStatus = "PAUSED"


These "PAUSED" strings should be the constant jobs.PauseStatusPaused.

Fixed. Is that another SDK name that should be changed, btw? :( That doesn't seem like a very idiomatic name.

pietern · 2023-07-05T19:52:05Z

cmd/bundle/deploy.go

+	b.Config.Bundle.Lock.Force = forceDeploy
+
+	if computeID == "" {
+		computeID = os.Getenv("DATABRICKS_CLUSTER_ID")


OK, computeID works. Can you remove the getenv check though? It will mean that deployment fails if that value is set if it's not in dev mode, which would be unexpected behavior to me. Having the flag alone captures the intent without that failure mode.

cmd/bundle/deploy.go

pietern

LGTM. See comment for failing test explanation.

bundle/config/mutator/process_environment_mode.go

bundle/config/bundle.go

* Add development runs ([#522](#522)). * Support tab completion for profiles ([#572](#572)). * Correctly use --profile flag passed for all bundle commands ([#571](#571)). * Disallow notebooks in paths where files are expected ([#573](#573)). * Improve auth login experience ([#570](#570)). * Remove base path checks during sync ([#576](#576)). * First look for databricks.yml before falling back to bundle.yml ([#580](#580)). * Integrate with auto-release infra ([#581](#581)). API Changes: * Removed `databricks metastores maintenance` command. * Added `databricks metastores enable-optimization` command. * Added `databricks tables update` command. * Changed `databricks account settings delete-personal-compute-setting` command with new required argument order. * Changed `databricks account settings read-personal-compute-setting` command with new required argument order. * Added `databricks clean-rooms` command group. OpenAPI SHA: 850a075ed9758d21a6bc4409506b48c8b9f93ab4, Date: 2023-07-18 Dependency updates: * Bump golang.org/x/term from 0.9.0 to 0.10.0 ([#567](#567)). * Bump golang.org/x/oauth2 from 0.9.0 to 0.10.0 ([#566](#566)). * Bump golang.org/x/mod from 0.11.0 to 0.12.0 ([#568](#568)). * Bump github.com/databricks/databricks-sdk-go from 0.12.0 to 0.13.0 ([#585](#585)).

lennartkats-db added 11 commits June 18, 2023 16:50

Add support for "debug runs"

48d6df3

- Add "mode: debug" property for environments - Add "--deploy", "--compute", "--no-wait" CLI flags

Introduce DATABRICKS_COMPUTE environment variable

27b2b77

Increase max concurrent runs for debug runs

823c868

Merge remote-tracking branch 'upstream/main' into add-debug-runs

6414701

Rename Deploy() to deploy()

bdd2124

Pause schedule for debug jobs

7c654ec

Merge remote-tracking branch 'upstream/main' into add-debug-runs

b7a80af

Rename to DATABRICKS_COMPUTE_ID

567d76d

Change to DATABRICKS_CLUSTER_ID

30667d4

Merge remote-tracking branch 'upstream/main' into add-debug-runs

b7c53b2

Dummy commit

458a2d0

lennartkats-db requested a review from pietern June 23, 2023 11:00

lennartkats-db mentioned this pull request Jun 23, 2023

Add support for "debug runs" #493

Closed

lennartkats-db commented Jun 27, 2023

View reviewed changes

pietern reviewed Jul 3, 2023

View reviewed changes

Rename "debug" to "development"

4762716

lennartkats-db changed the title ~~Add debug runs~~ Add development runs Jul 3, 2023

lennartkats-db added 6 commits July 3, 2023 16:47

Implement --no-wait for pipelines

15396bc

Add docstring

5a7b556

Pass compute override as part of the bundle object

34a1698

Don't support databricks bundle run --deploy for now

ddcd613

Merge remote-tracking branch 'databricks/main' into add-debug-runs

64edf39

Use latest config names

35f4d3b

lennartkats-db mentioned this pull request Jul 3, 2023

[DONTMERGE] Add databricks run --deploy #542

Closed

lennartkats-db requested a review from pietern July 3, 2023 16:04

Fix tests

36a6e93

pietern reviewed Jul 5, 2023

View reviewed changes

lennartkats-db added 3 commits July 7, 2023 09:38

Improve DATABRICKS_CLUSTER_ID handling for non-development modes

be2dd4f

Make it possible to override the compute id in an environment

d221d4d

Cleanup

50b76bb

Cleanup

6b221c0

lennartkats-db force-pushed the add-debug-runs branch from 529ab3f to 6b221c0 Compare July 7, 2023 09:12

Support experiment names that use full paths

cbd3068

pietern reviewed Jul 7, 2023

View reviewed changes

bundle/config/mutator/process_environment_mode.go Outdated Show resolved Hide resolved

bundle/config/bundle.go Show resolved Hide resolved

lennartkats-db added 2 commits July 7, 2023 18:04

Fix paths on Windows

54a66c0

Add comment

77fcc92

lennartkats-db merged commit 57e75d3 into databricks:main Jul 12, 2023
4 checks passed

lennartkats-db deleted the add-debug-runs branch July 12, 2023 06:51

nfx mentioned this pull request Jul 18, 2023

Release v0.201.0 #586

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add development runs #522

Add development runs #522

lennartkats-db commented Jun 23, 2023 •

edited

Loading

lennartkats-db Jun 23, 2023

pietern Jul 3, 2023

lennartkats-db Jul 3, 2023

pietern Jul 5, 2023

lennartkats-db Jul 7, 2023

pietern Jul 3, 2023

pietern Jul 5, 2023

pietern Jul 5, 2023

pietern Jul 5, 2023

lennartkats-db Jul 7, 2023

pietern Jul 5, 2023

lennartkats-db Jul 7, 2023

pietern Jul 5, 2023

lennartkats-db Jul 7, 2023 •

edited

Loading

pietern Jul 5, 2023

pietern left a comment

Add development runs #522

Add development runs #522

Conversation

lennartkats-db commented Jun 23, 2023 • edited Loading

bundle.yml changes

CLI changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lennartkats-db Jul 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pietern left a comment

Choose a reason for hiding this comment

lennartkats-db commented Jun 23, 2023 •

edited

Loading

lennartkats-db Jul 7, 2023 •

edited

Loading