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.

Sign up for GitHub

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

Jump to bottom

Reduce time until the prompt is shown for bundle run #1727

Merged
merged 4 commits into from
Sep 21, 2024
Merged

Reduce time until the prompt is shown for bundle run #1727

merged 4 commits into from
Sep 21, 2024

Conversation

lennartkats-db
Copy link
Contributor

@lennartkats-db lennartkats-db commented Aug 28, 2024 •
edited
Loading

Summary

Makes the databricks bundle run command use local state before showing the menu prompt, which makes it show more quickly. For large/busy workspaces this means the prompt can show 2-3 seconds earlier.

ilia-db
ilia-db reviewed Aug 28, 2024
View reviewed changes
cmd/bundle/run.go Outdated Show resolved Hide resolved
pietern
pietern reviewed Aug 28, 2024
View reviewed changes
Copy link
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

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

Removing the state pull saves only 1 call.

If there is something that makes this slow, I suspect it is something else.

IIRC you care about the latency prior to the prompt here, and nothing else, correct?

cmd/bundle/run.go Outdated Show resolved Hide resolved
@lennartkats-db
Copy link
Contributor Author

@pietern

Removing the state pull saves only 1 call.

If there is something that makes this slow, I suspect it is something else.

IIRC you care about the latency prior to the prompt here, and nothing else, correct?

Yeah this is the behavior we discussed the other day. I think you mentioned you expected it to use the cache, but it does not. The change here makes a difference of multiple seconds before the prompt shows.

@lennartkats-db
Copy link
Contributor Author

Why only for big workspaces? There shouldn't be anything in there that gets slower.

For big/busy workspaces, APIs like the workspace API tend to be slower to respond. The difference is less pronounced on workspaces that aren't as busy.

pietern
pietern requested changes Sep 11, 2024
View reviewed changes
cmd/bundle/run.go Outdated Show resolved Hide resolved
pietern
pietern reviewed Sep 11, 2024
View reviewed changes
Copy link
Contributor

@pietern pietern left a comment

Choose a reason for hiding this comment

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

The change LGTM. Can you update the PR title and summary to reflect what the change does now (and quantify the latency improvement if possible)?

@lennartkats-db lennartkats-db changed the title Speed up bundle run command Speed up bundle run command by using cached state before showing menu prompt Sep 18, 2024
@pietern
Copy link
Contributor

pietern commented Sep 19, 2024

@lennartkats-db This is not using state caching, nor does it make bundle run faster.

A more accurate title would be "Reduce time until the prompt is shown for bundle run."

It doesn't change the runtime if the user doesn't use the prompt.

@lennartkats-db lennartkats-db changed the title Speed up bundle run command by using cached state before showing menu prompt Reduce time until the prompt is shown for bundle run Sep 19, 2024
@lennartkats-db
Copy link
Contributor Author

That works. I really had local state in mind rather than "cached" state. I'm not sure it really matters for customers though.

@lennartkats-db lennartkats-db added this pull request to the merge queue Sep 21, 2024
Merged via the queue into databricks:main with commit 6c57683 Sep 21, 2024
5 checks passed
@lennartkats-db lennartkats-db deleted the bundle-run-cache branch September 21, 2024 06:43
andrewnester added a commit that referenced this pull request Oct 1, 2024
@andrewnester
[Release] Release v0.229.0
00a9724 
Bundles:
 * Added support for creating all-purpose clusters ([#1698](#1698)).
 * Reduce time until the prompt is shown for bundle run ([#1727](#1727)).
 * Use Unity Catalog for pipelines in the default-python template ([#1766](#1766)).
 * Add verbose flag to the "bundle deploy" command ([#1774](#1774)).
 * Fixed full variable override detection ([#1787](#1787)).
 * Add sub-extension to resource files in built-in templates ([#1777](#1777)).
 * Fix panic in `apply_presets.go` ([#1796](#1796)).

Internal:
 * Assert tokens are redacted in origin URL when username is not specified ([#1785](#1785)).
 * Refactor jobs path translation ([#1782](#1782)).
 * Add JobTaskClusterSpec validate mutator ([#1784](#1784)).
 * Pin Go toolchain to 1.22.7 ([#1790](#1790)).
 * Modify SetLocation test utility to take full locations as argument ([#1788](#1788)).
 * Simplified isFullVariableOverrideDef implementation ([#1791](#1791)).
 * Sort tasks by `task_key` before generating the Terraform configuration ([#1776](#1776)).
 * Trim trailing whitespace ([#1794](#1794)).
 * Move trampoline code into trampoline package ([#1793](#1793)).
 * Rename `RootPath` -> `BundleRootPath` ([#1792](#1792)).

API Changes:
 * Changed `databricks apps delete` command to return .
 * Changed `databricks apps deploy` command with new required argument order.
 * Changed `databricks apps start` command to return .
 * Changed `databricks apps stop` command to return .
 * Added `databricks temporary-table-credentials` command group.
 * Added `databricks serving-endpoints put-ai-gateway` command.
 * Added `databricks disable-legacy-access` command group.
 * Added `databricks account disable-legacy-features` command group.

OpenAPI commit 6f6b1371e640f2dfeba72d365ac566368656f6b6 (2024-09-19)
Dependency updates:
 * Upgrade to Go SDK 0.47.0 ([#1799](#1799)).
 * Upgrade to TF provider 1.52 ([#1781](#1781)).
 * Bump golang.org/x/mod from 0.20.0 to 0.21.0 ([#1758](#1758)).
 * Bump github.com/hashicorp/hc-install from 0.7.0 to 0.9.0 ([#1772](#1772)).
@andrewnester andrewnester mentioned this pull request Oct 1, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Oct 1, 2024
@andrewnester
[Release] Release v0.229.0 (#1801)
c28d64f 
Bundles:
* Added support for creating all-purpose clusters
([#1698](#1698)).
* Reduce time until the prompt is shown for bundle run
([#1727](#1727)).
* Use Unity Catalog for pipelines in the default-python template
([#1766](#1766)).
* Add verbose flag to the "bundle deploy" command
([#1774](#1774)).
* Fixed full variable override detection
([#1787](#1787)).
* Add sub-extension to resource files in built-in templates
([#1777](#1777)).
* Fix panic in `apply_presets.go`
([#1796](#1796)).

Internal:
* Assert tokens are redacted in origin URL when username is not
specified ([#1785](#1785)).
* Refactor jobs path translation
([#1782](#1782)).
* Add JobTaskClusterSpec validate mutator
([#1784](#1784)).
* Pin Go toolchain to 1.22.7
([#1790](#1790)).
* Modify SetLocation test utility to take full locations as argument
([#1788](#1788)).
* Simplified isFullVariableOverrideDef implementation
([#1791](#1791)).
* Sort tasks by `task_key` before generating the Terraform configuration
([#1776](#1776)).
* Trim trailing whitespace
([#1794](#1794)).
* Move trampoline code into trampoline package
([#1793](#1793)).
* Rename `RootPath` -> `BundleRootPath`
([#1792](#1792)).

API Changes:
 * Changed `databricks apps delete` command to return .
* Changed `databricks apps deploy` command with new required argument
order.
 * Changed `databricks apps start` command to return .
 * Changed `databricks apps stop` command to return .
 * Added `databricks temporary-table-credentials` command group.
 * Added `databricks serving-endpoints put-ai-gateway` command.
 * Added `databricks disable-legacy-access` command group.
 * Added `databricks account disable-legacy-features` command group.

OpenAPI commit 6f6b1371e640f2dfeba72d365ac566368656f6b6 (2024-09-19)
Dependency updates:
* Upgrade to Go SDK 0.47.0
([#1799](#1799)).
* Upgrade to TF provider 1.52
([#1781](#1781)).
* Bump golang.org/x/mod from 0.20.0 to 0.21.0
([#1758](#1758)).
* Bump github.com/hashicorp/hc-install from 0.7.0 to 0.9.0
([#1772](#1772)).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pietern pietern pietern approved these changes

@ilia-db ilia-db Awaiting requested review from ilia-db

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lennartkats-db @pietern @ilia-db