Refactor `StateStore` to make keys more explicit at the point of use. by kartikgupta-db · Pull Request #913 · databricks/databricks-vscode

kartikgupta-db · 2023-10-23T10:08:46Z

Changes

Advantages of this approach

Adding new variables does not require creating an explicit setter and getter. Default setters and getters should work out of the box.
Easier to see all the information in 1 location.
The keys are immutably exposed to point of use. It allows for potential programatic access to these functions.
Setters require calling the set function, which is async. This prevents us from having to create even more functions just to make setting a variable awaitable.

Tests

…undle-overrides

ilia-db · 2023-10-23T11:26:29Z

packages/databricks-vscode/src/vscode-objs/StateStorage.ts

+type GetterReturnType<D extends KeyInfo<any>> = D extends {getter: infer G}
+    ? G extends (...args: any[]) => any
+        ? ReturnType<G>
+        : undefined


never might be a better type here?

We want the final return type of the get function to be undefined if both getter and default value are undefined.

Yeah, but that's the undefined on the line below. This undefined will happen when a default value is undefined and a non-undefined getter field is not a function (e.g. doesn't match G extends (...args: any[]) => any) - that seems as something that should never happen. Might be misunderstanding something though

KeyInfo shape should prevent this from happening, so this line should be a noop either way. Semantically never does make more sense. I will fix it in another PR.

ilia-db · 2023-10-23T12:10:54Z

packages/databricks-vscode/src/vscode-objs/StateStorage.ts

+function withType<V>() {
+    return function <D extends KeyInfo<V>>(data: D) {


Why nested functions? Can outer function already return data as KeyInfoWithType<V>? Later in the storage.get you won't need to use that as anymore.

This is related to how I wanted getters to work. Replied here

ilia-db · 2023-10-23T12:20:09Z

packages/databricks-vscode/src/vscode-objs/StateStorage.ts

+};
+
+type ValueType<K extends keyof typeof Keys> = (typeof Keys)[K]["_type"];
+type GetterReturnType<D extends KeyInfo<any>> = D extends {getter: infer G}


Does it make sense for a getter type to be different from ValueType?

Getters can return ValueType | undefined or just ValueType. I wanted to account for both the cases, and have the autocompletion NOT complete return as undefined if the getter always returns a value or if the defaultValue is defined.

This is why we also require the nested withType function. This allows us to have getters which extend the standard getter type in the KeyInfo and potentially NOT return undefined. If we don't do this, we will have to type data as any (makes defining new keys type unsafe) or we will lose out on this information about getters.

ilia-db · 2023-10-23T12:23:27Z

packages/databricks-vscode/src/vscode-objs/StateStorage.ts

+        ? ValueType<K>
+        : GetterReturnType<(typeof Keys)[K]>;
+
+type KeyInfoWithType<V> = KeyInfo<V> & {


What are the benefits of keeping it separate from the KeyInfo? Can those additional types just be defined on the KeyInfo, for the simplicity sake?

KeyInfo holds actual concrete data. The type fields are only types. Defining them on keyinfo means someone can try setting them when adding a new fields to keys.

## packages/databricks-vscode ## <small>1.2.3 (2023-11-06)</small> * Make configuration wizard sticky (#925) ([7a4fb31](7a4fb31)), closes [#925](#925) * Refactor `StateStore` to make keys more explicit at the point of use. (#913) ([5b8fb23](5b8fb23)), closes [#913](#913) * Use databricks CLI log-file option to capture the logs (#923) ([18283bb](18283bb)), closes [#923](#923) ## packages/databricks-vscode-types ## <small>1.2.3 (2023-11-06)</small> --------- Co-authored-by: releasebot <[email protected]> Co-authored-by: Kartik Gupta <[email protected]> Co-authored-by: kartikgupta-db <[email protected]>

## packages/databricks-vscode ## (2024-03-07) * Refactor configuration UI to better support override and bundle configuration model (#940) ([b5a80a7](b5a80a7)), closes [#940](#940) * Add a model for remote state (`BundleRemoteStateModel`) (#1006) ([125a2ec](125a2ec)), closes [#1006](#1006) * Add a note to our quick start (#1126) ([93cbbe5](93cbbe5)), closes [#1126](#1126) * Add a view for DABs resource viewer (#1002) ([b853df6](b853df6)), closes [#1002](#1002) * Add bundle project manager (#1011) ([acfa61a](acfa61a)), closes [#1011](#1011) * Add config level change events in `ConfigModel` (#937) ([84e2198](84e2198)), closes [#937](#937) * Add DATABRICKS_CLI_UPSTREAM env var to CLI calls (#1112) ([8c60fa5](8c60fa5)), closes [#1112](#1112) * Add decoration provider for decorating treeView items with modified status (#1035) ([863162f](863162f)), closes [#1035](#1035) * Add e2e tests for bundle init flow (#1073) ([efa3c6b](efa3c6b)), closes [#1073](#1073) * Add e2e tests for standalone running flows (#1085) ([168d69f](168d69f)), closes [#1085](#1085) * Add full UI support for supported auth types loaded from profiles (#1061) ([af29a83](af29a83)), closes [#1061](#1061) * Add icons for more task types (#1030) ([3682c76](3682c76)), closes [#1030](#1030) * Add telemetry for bundle init flow (#1117) ([ee955d4](ee955d4)), closes [#1117](#1117) * Add telemetry for the v2 (#1105) ([12f306a](12f306a)), closes [#1105](#1105) * Add UI to configure PAT auth (#1033) ([2533aec](2533aec)), closes [#1033](#1033) * Added functionality to deploy the current bundle (#1010) ([936fc49](936fc49)), closes [#1010](#1010) * All `AuthProviders` must implement a non silent `check`. (#939) ([41f076c](41f076c)), closes [#939](#939) * Allow configuring current project with minimal config (#1039) ([385d1fc](385d1fc)), closes [#1039](#1039) * Allow right click -> copy for all tree items (#1101) ([ab214b4](ab214b4)), closes [#1101](#1101) * Append CLI path to make sure it is picked up last (#1121) ([53debe9](53debe9)), closes [#1121](#1121) * Append profile to databrickscfg instead of writing the full file (#1060) ([2af50c6](2af50c6)), closes [#1060](#1060) * Automatically read auth info from override, bundle and .databrickscfg, in order (#995) ([956a9e6](956a9e6)), closes [#995](#995) * Backend for reading and writing overrides for bundle config (#901) ([157fb6f](157fb6f)), closes [#901](#901) * Bring missing changes from the main branch (#1093) ([e2adc6d](e2adc6d)), closes [#1093](#1093) * Bump CLI to 0.215.0 (#1119) ([c121b60](c121b60)), closes [#1119](#1119) * Bump databricks-cli to v0.212.3 (#1037) ([661b66f](661b66f)), closes [#1037](#1037) * Bump to 2.0.0 (#1049) ([2dcf080](2dcf080)), closes [#1049](#1049) * Bundle integ merge main (#1009) ([d8852bd](d8852bd)), closes [#1009](#1009) * Bundle watcher (#892) ([559911c](559911c)), closes [#892](#892) * Cancel run in the UI when CLI fails (#1102) ([71303b8](71303b8)), closes [#1102](#1102) * Check for idle deployment state and not running (#1064) ([f2f0126](f2f0126)), closes [#1064](#1064) * Create profile wizard for Azure-Cli and OAuth (U2M) auth types (#990) ([263f731](263f731)), closes [#990](#990) * Deeplink to the PAT creation page in PAT profile creation wizard (#1047) ([fc9d3e7](fc9d3e7)), closes [#1047](#1047) * Delete yaml from files watched by bundle autocomplete, when the yaml is deleted (#931) ([76d7d37](76d7d37)), closes [#931](#931) [/github.com//pull/922#discussion_r1382076227](https://github.com//github.com/databricks/databricks-vscode/pull/922/issues/discussion_r1382076227) * Deploy and run resource (#1012) ([9dcbd87](9dcbd87)), closes [#1012](#1012) * Deploy bundle before running files using remote run modes (#1019) ([a55eb15](a55eb15)), closes [#1019](#1019) * Dissallow all overrides except cluster ID for dev mode and Auth Params for all modes (#972) ([837d3f2](837d3f2)), closes [#972](#972) * Don't allow per file run modes in prod/staging target (#1032) ([5e44b1f](5e44b1f)), closes [#1032](#1032) * Don't show deeplinks for undeployed resources (#1043) ([62a47e3](62a47e3)), closes [#1043](#1043) * Expose bundle-init wizard in empty workspace (#1034) ([4f8ece7](4f8ece7)), closes [#1034](#1034) * Fix "Cluster not found" error when attaching cluster on target change (#1003) ([ab4e603](ab4e603)), closes [#1003](#1003) * Fix bundle-init on windows (#1058) ([626e134](626e134)), closes [#1058](#1058) * Fix bundle-run, use logging env vars (#1048) ([26e03ad](26e03ad)), closes [#1048](#1048) * Fix deeplinks format to work across all clouds (#1044) ([0f85446](0f85446)), closes [#1044](#1044) * Fix regression where bundle logs are not displayed during deploy (#1086) ([9e05703](9e05703)), closes [#1086](#1086) * Fix regression where switching targets, with the same host, does not trigger a model refresh (#1088) ([d5cfc5e](d5cfc5e)), closes [#1088](#1088) * Hide authentication quickpicks when checking auth provider (#1040) ([76c33fd](76c33fd)), closes [#1040](#1040) * Hide context buttons during deployment (#1062) ([927ea42](927ea42)), closes [#1062](#1062) * Improve bundle init (#1041) ([3bb953e](3bb953e)), closes [#1041](#1041) * Improve in-progress auth UI (#1038) ([6f6fd6f](6f6fd6f)), closes [#1038](#1038) * Improve login flow, properly init cluster manager ([772d112](772d112)) * Improve login wizard (#1045) ([25f964f](25f964f)), closes [#1045](#1045) * Improve UX for bundle init folder selection (#1026) ([0035539](0035539)), closes [#1026](#1026) * Initialise -> Initialize (#1051) ([4351936](4351936)), closes [#1051](#1051) * Inject env vars into python unit test debugging sessions (#1120) ([264cb54](264cb54)), closes [#1120](#1120) * Limit onError usages to high level functions (#999) ([cac9604](cac9604)), closes [#999](#999) * Listen to changes to "host" in databricks.yml to retrigger login (#1057) ([8471909](8471909)), closes [#1057](#1057) * Make all models inherit from a common base class (#994) ([7acb093](7acb093)), closes [#994](#994) * Make DABs autocomplete detect new bundle files (#922) ([f356219](f356219)), closes [#922](#922) * Merge branch 'main' of github.com:databricks/databricks-vscode into bundle-integ (#991) ([3c578aa](3c578aa)), closes [#991](#991) * Migrate from project.json to databricks.yml (#1013) ([ba2327d](ba2327d)), closes [#1013](#1013) * Minor fixes for `configurationView` (#1031) ([9f16b58](9f16b58)), closes [#1031](#1031) * Move all dbconnect run options to Databricks Run Icon (#1066) ([c28620a](c28620a)), closes [#1066](#1066) * Move existing run options to new Databricks Run icon (#968) ([717c94d](717c94d)), closes [#968](#968) * Move openDatabricksConfigFile to utils (#1067) ([e5f0780](e5f0780)), closes [#1067](#1067) * Pass CLI path to all auth providers and login only once during bundle init (#1114) ([c6e9b0e](c6e9b0e)), closes [#1114](#1114) * Prevent running resources when cli already launched but no run status (#1072) ([7412d95](7412d95)), closes [#1072](#1072) * Read data from global workspace block in databaricks.yml (#1059) ([fd8ae92](fd8ae92)), closes [#1059](#1059) * Redirect summarise and validate errors to bundle output panel (#1076) ([65f03ee](65f03ee)), closes [#1076](#1076) * Refactor `StateStore` to make keys more explicit at the point of use. (#913) ([5b8fb23](5b8fb23)), closes [#913](#913) * Refactor config loaders (#978) ([8ce4c9a](8ce4c9a)), closes [#978](#978) * Remove bundle source (#1005) ([ea3ed45](ea3ed45)), closes [#1005](#1005) * Remove sync destination logic (#1096) ([bb63006](bb63006)), closes [#1096](#1096) * Remove unused sync code (#1087) ([83da2b5](83da2b5)), closes [#1087](#1087) * Replace `JSON.stringify` based object comparisions with `lodash.isEqual` (#943) ([3d7abe3](3d7abe3)), closes [#943](#943) * Return config source from `ConfigModel` (#941) ([9a32556](9a32556)), closes [#941](#941) * Revert "Trigger target change on prevalidate changes, so that auth resolution happens again" (#1056) ([c4f25a2](c4f25a2)), closes [#1056](#1056) [#1055](#1055) * Send environment type (tests, prod) in telemetry events (#944) ([09a59a3](09a59a3)), closes [#944](#944) * Set authprovider when logging out (#1042) ([5b1eb9b](5b1eb9b)), closes [#1042](#1042) * Setup pre-release CI for v2 (#1109) ([5b3fbd3](5b3fbd3)), closes [#1109](#1109) * Show a more actionable error message when deployment fails (#1050) ([f012ee5](f012ee5)), closes [#1050](#1050) * Show deploy and run commands in terminal (#1020) ([2cb93aa](2cb93aa)), closes [#1020](#1020) * Show resources in DABS Resource Explorer (#1008) ([f6dcb48](f6dcb48)), closes [#1008](#1008) * Show run status for jobs and pipeline (#1017) ([ad72331](ad72331)), closes [#1017](#1017) * Show t&c popup for private preview (#1128) ([d5b69e4](d5b69e4)), closes [#1128](#1128) * State message tooltips (#1100) ([f0986e1](f0986e1)), closes [#1100](#1100) * Take a hard dependency on redhat.vscode-yaml extension (#1127) ([f3354a7](f3354a7)), closes [#1127](#1127) * Trigger target change on prevalidate changes, so that auth resolution happens again (#1055) ([6c00c17](6c00c17)), closes [#1055](#1055) * Update CLI to v0.214.1 (#1099) ([ea7b079](ea7b079)), closes [#1099](#1099) * Update dependencies (#974) ([1835fc2](1835fc2)), closes [#974](#974) * Update JS SDK to 0.6.1 (#1053) ([847ce72](847ce72)), closes [#1053](#1053) * Update login wizard and show cancel button for running resources (#1071) ([fa4013c](fa4013c)), closes [#1071](#1071) * Update NOTICE.md (#1115) ([8d60383](8d60383)), closes [#1115](#1115) * Use `bundle validate` to load interpolated view of configs after login (#979) ([6b2caca](6b2caca)), closes [#979](#979) * Use a tree representation to structure resource explorer UI components (#1029) ([a3c5f14](a3c5f14)), closes [#1029](#1029) * Use bundle for configuration and not project.json (#924) ([29474b3](29474b3)), closes [#924](#924) * Use raw app insights keys for the telemetry reporter (#1116) ([18bb45b](18bb45b)), closes [#1116](#1116) * Use sync icon for sync destination and make it green (#1054) ([5da4b76](5da4b76)), closes [#1054](#1054) ## packages/databricks-vscode-types ## (2024-03-07) * Bump to 2.0.0 (#1049) ([2dcf080](2dcf080)), closes [#1049](#1049) * Bundle integ merge main (#1009) ([d8852bd](d8852bd)), closes [#1009](#1009) * Merge branch 'main' of github.com:databricks/databricks-vscode into bundle-integ (#991) ([3c578aa](3c578aa)), closes [#991](#991) * Update dependencies (#974) ([1835fc2](1835fc2)), closes [#974](#974) * Use bundle for configuration and not project.json (#924) ([29474b3](29474b3)), closes [#924](#924) --------- Co-authored-by: releasebot <[email protected]> Co-authored-by: kartikgupta-db <[email protected]>

kartikgupta-db added 3 commits October 23, 2023 11:50

wip

c839c00

Merge branch 'main' of github.com:databricks/databricks-vscode into b…

6b9454f

…undle-overrides

fixes

933887a

kartikgupta-db temporarily deployed to azure-prod-usr October 23, 2023 10:10 — with GitHub Actions Inactive

kartikgupta-db requested a review from ilia-db October 23, 2023 10:19

remove unused telemetry

0e2f467

kartikgupta-db temporarily deployed to azure-prod-usr October 23, 2023 10:20 — with GitHub Actions Inactive

kartikgupta-db enabled auto-merge (squash) October 23, 2023 10:36

kartikgupta-db disabled auto-merge October 23, 2023 10:36

ilia-db reviewed Oct 23, 2023

View reviewed changes

ilia-db approved these changes Oct 23, 2023

View reviewed changes

Merge branch 'main' into state-store-refactor

b173b1d

kartikgupta-db enabled auto-merge (squash) October 23, 2023 13:01

kartikgupta-db temporarily deployed to azure-prod-usr October 23, 2023 13:01 — with GitHub Actions Inactive

kartikgupta-db merged commit 5b8fb23 into main Oct 23, 2023

kartikgupta-db deleted the state-store-refactor branch October 23, 2023 14:00

github-actions bot mentioned this pull request Nov 6, 2023

Release: v1.2.3 #930

Merged

This was referenced Mar 5, 2024

Release: v2.0.0-preview #1110

Closed

Release: v2.0.1-preview #1130

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor `StateStore` to make keys more explicit at the point of use. #913

Refactor `StateStore` to make keys more explicit at the point of use. #913
kartikgupta-db merged 5 commits intomainfrom
state-store-refactor

kartikgupta-db commented Oct 23, 2023

Uh oh!

ilia-db Oct 23, 2023

Uh oh!

kartikgupta-db Oct 23, 2023

Uh oh!

ilia-db Oct 23, 2023

Uh oh!

kartikgupta-db Oct 23, 2023

Uh oh!

ilia-db Oct 23, 2023

Uh oh!

kartikgupta-db Oct 23, 2023

Uh oh!

ilia-db Oct 23, 2023

Uh oh!

kartikgupta-db Oct 23, 2023

Uh oh!

ilia-db Oct 23, 2023

Uh oh!

kartikgupta-db Oct 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		function withType<V>() {
		return function <D extends KeyInfo<V>>(data: D) {

Conversation

kartikgupta-db commented Oct 23, 2023

Changes

Tests

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants