Add support for reprompts if user input does not match template schema #946

shreyas-goenka · 2023-11-03T16:07:19Z

Changes

This PR adds retry logic to user input prompts, prompting users again if the value does not match the requirements specified in the bundle template schema.

Tests

Manually. Here's an example UX. The first prompt expects an integer and the second one a string made only from the letters "defg"

shreyas.goenka@THW32HFW6T cli % cli bundle init ~/mlops-stack

Please enter an integer [123]: abc
Validation failed: "abc" is not a integer

Please enter an integer [123]: 123

Please enter a string [dddd]: apple
Validation failed: invalid value for input_root_dir: "apple". Only characters the 'd', 'e', 'f', 'g' are allowed

…hods

pietern

The retry loop should be done in the promptForValues function to reduce duplication. The two different prompt functions can return a proper error or a retryable error and depending on the type can the loop decide to retry or not.

libs/template/config.go

pietern · 2023-11-03T17:41:36Z

libs/template/config.go

-		// Validate the partial config based on this update
-		if err := c.schema.ValidateInstance(c.values); err != nil {
+		// Validate the partial config based on this new value by the user
+		err = c.schema.ValidateInstance(c.values)


This is duplicated in the other prompt function.

Not really duplicated right? For promptText this is wrapped in a retry loop.

So for promptSelect we never retry if it fails?

Done. Moved retry loop to the promptForValues function. It's probably good enough to retry on all validation errors. WDYT?

pietern · 2023-11-03T17:45:58Z

W.r.t. the error message:

Validation failed: could not parse "abc" as a integer: strconv.ParseInt: parsing "abc": invalid syntax

This exposes internals. A more friendly error message would be:

Validation failed: "abc" is not an integer.

shreyas-goenka · 2023-12-05T09:27:54Z

Output logs after addressing comments:

shreyas.goenka@THW32HFW6T mlops-stack % cli bundle init .

Please enter an integer [123]: abcd
Validation failed: "abcd" is not a integer

Please enter an integer [123]: 123

Please enter a string [dddd]: qwerty
Validation failed: invalid value for input_root_dir: "qwerty". Only characters the 'd', 'e', 'f', 'g' are allowed

Template schema for these properties:

  "properties": {
    "input_project_name": {
      "order": 1,
      "type": "integer",
      "default": 123,
      "description": "\nPlease enter an integer"
    },
    "input_root_dir": {
      "order": 2,
      "type": "string",
      "default": "dddd",
      "description": "\nPlease enter a string",
      "pattern": "^[d-g]+$",
      "pattern_match_failure_message": "Only characters the 'd', 'e', 'f', 'g' are allowed"
    },

libs/jsonschema/utils.go

libs/template/config.go

pietern · 2023-12-06T09:05:34Z

libs/template/config.go

-		// Validate the partial config based on this update
-		if err := c.schema.ValidateInstance(c.values); err != nil {
+		// Validate the partial config based on this new value by the user
+		err = c.schema.ValidateInstance(c.values)


So for promptSelect we never retry if it fails?

pietern · 2023-12-06T09:10:23Z

The PR description is out of date from the latest behavior.

libs/template/config.go

CLI: * Tweak command groups in CLI help ([#1092](#1092)). * Fixed storage-credentials list command in text output ([#1094](#1094)). * Allow account client auth with environment variables when no `.databrickscfg` file present ([#1097](#1097)). * Always log with text format by default ([#1096](#1096)). Bundles: * Added exec.NewCommandExecutor to execute commands with correct interpreter ([#1075](#1075)). * Use resource key as name in permissions code ([#1087](#1087)). * Add support for reprompts if user input does not match template schema ([#946](#946)). * Improve experience for multiple builtin templates ([#1052](#1052)). * Improve error when bundle root is not writable ([#1093](#1093)). * Change recommended production deployment path from `/Shared` to `/Users` ([#1091](#1091)). * Allow referencing bundle resources by name ([#872](#872)). * Make libs/exec fallback to `sh` if `bash` cannot be found ([#1114](#1114)). Internal: * Functionality to walk a `config.Value` tree ([#1081](#1081)). * Rename libs/config -> libs/dyn ([#1086](#1086)). * Added codecov integration on code push ([#1095](#1095)). * Consolidate functions to convert `dyn.Value` to native types ([#1100](#1100)). * Define constant for the invalid `dyn.Value` ([#1101](#1101)). API Changes: * Changed `databricks serving-endpoints update-config` command with new required argument order. * Changed `databricks account ip-access-lists create` command with new required argument order. * Changed `databricks account ip-access-lists replace` command with new required argument order. * Changed `databricks account ip-access-lists update` command with new required argument order. * Changed `databricks ip-access-lists create` command with new required argument order. * Changed `databricks ip-access-lists replace` command with new required argument order. * Changed `databricks ip-access-lists update` command with new required argument order. OpenAPI commit a7a9dc025bb80303e676bf3708942c6aa06689f1 (2024-01-04) Dependency updates: * Bump github.com/hashicorp/terraform-exec from 0.19.0 to 0.20.0 ([#1088](#1088)). * Bump go.uber.org/mock from 0.3.0 to 0.4.0 ([#1089](#1089)). * Bump github.com/hashicorp/terraform-json from 0.18.0 to 0.20.0 ([#1090](#1090)). * Bump golang.org/x/term from 0.15.0 to 0.16.0 ([#1103](#1103)). * Bump github.com/cloudflare/circl from 1.3.3 to 1.3.7 ([#1105](#1105)). * Bump golang.org/x/sync from 0.5.0 to 0.6.0 ([#1104](#1104)). * Upgrade golang.org/x/crypto to v0.17.0 in internal module ([#1110](#1110)). * Upgrade Go SDK to v0.29.0 ([#1111](#1111)).

shreyas-goenka added 5 commits October 31, 2023 17:38

Make to/from string methods private to the jsonschema package

3eec419

-

f765a16

-

e0b178b

Merge remote-tracking branch 'origin' into refactor-schema-string-met…

0eb5020

…hods

Add reprompting when user input does not match template schema

1c0b074

shreyas-goenka changed the base branch from main to refactor-schema-string-methods November 3, 2023 16:09

shreyas-goenka requested review from pietern and andrewnester November 3, 2023 16:11

pietern reviewed Nov 3, 2023

View reviewed changes

Merge remote-tracking branch 'origin' into repromt-on-validation-error

abbe612

Base automatically changed from refactor-schema-string-methods to main November 6, 2023 15:12

shreyas-goenka added 5 commits November 6, 2023 17:50

friendly error messages

e1401f7

infinite retries

e8a3d49

merge

cd0547f

cleanup

d1c2776

an -> a

3fad026

shreyas-goenka requested a review from pietern November 6, 2023 17:08

merge

da51c73

shreyas-goenka removed request for pietern and andrewnester December 4, 2023 11:40

shreyas-goenka added 3 commits December 5, 2023 14:39

small rename

e59a30f

refine error message

0d24e39

rename var

a3dfe0b

shreyas-goenka requested review from pietern and andrewnester December 5, 2023 09:31

pietern reviewed Dec 6, 2023

View reviewed changes

Merge remote-tracking branch 'origin' into repromt-on-validation-error

0852e42

shreyas-goenka added 12 commits December 6, 2023 15:38

PatternMatchFailedError -> PatternMatchError

03ca489

indicated -> indicates

7070cf7

remove parse comment at the wrong place

7b283e0

Merge remote-tracking branch 'origin' into repromt-on-validation-error

46d45dd

remove custom catching of the error

eb55055

PatternMatchError -> patternMatchError

5dff694

add comment

b8e4a73

comments

b98af23

add retry for selection UI

90e2d1d

-

a5b839c

-

c0ec638

-

9defc3d

shreyas-goenka requested a review from pietern December 11, 2023 22:50

andrewnester reviewed Dec 13, 2023

View reviewed changes

libs/template/config.go Show resolved Hide resolved

refactor to promptOnce

61f5354

shreyas-goenka requested a review from andrewnester December 22, 2023 14:14

andrewnester approved these changes Dec 22, 2023

View reviewed changes

libs/template/config.go Outdated Show resolved Hide resolved

libs/template/config.go Outdated Show resolved Hide resolved

shreyas-goenka added 2 commits December 22, 2023 20:08

Address comment

2ecdf4e

-

effce0f

shreyas-goenka added this pull request to the merge queue Dec 22, 2023

Merged via the queue into main with commit f2408ed Dec 22, 2023
4 checks passed

shreyas-goenka deleted the repromt-on-validation-error branch December 22, 2023 15:49

pietern mentioned this pull request Jan 11, 2024

Release v0.212.0 #1116

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for reprompts if user input does not match template schema #946

Add support for reprompts if user input does not match template schema #946

shreyas-goenka commented Nov 3, 2023 •

edited

Loading

pietern left a comment

pietern Nov 3, 2023

shreyas-goenka Nov 6, 2023

pietern Dec 6, 2023

shreyas-goenka Dec 11, 2023

pietern commented Nov 3, 2023

shreyas-goenka commented Dec 5, 2023 •

edited

Loading

pietern Dec 6, 2023

pietern commented Dec 6, 2023

Add support for reprompts if user input does not match template schema #946

Add support for reprompts if user input does not match template schema #946

Conversation

shreyas-goenka commented Nov 3, 2023 • edited Loading

Changes

Tests

pietern left a comment

Choose a reason for hiding this comment

pietern Nov 3, 2023

Choose a reason for hiding this comment

shreyas-goenka Nov 6, 2023

Choose a reason for hiding this comment

pietern Dec 6, 2023

Choose a reason for hiding this comment

shreyas-goenka Dec 11, 2023

Choose a reason for hiding this comment

pietern commented Nov 3, 2023

shreyas-goenka commented Dec 5, 2023 • edited Loading

pietern Dec 6, 2023

Choose a reason for hiding this comment

pietern commented Dec 6, 2023

shreyas-goenka commented Nov 3, 2023 •

edited

Loading

shreyas-goenka commented Dec 5, 2023 •

edited

Loading