Refine mode: production diagnostic output
#2236
Merged
+21
−6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lennartkats-db
requested review from
pietern,
andrewnester,
shreyas-goenka and
denik
as code owners
lennartkats-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
lennartkats-db
commented
Jan 27, 2025
| { | ||
| Severity: diag.Error, | ||
| Summary: "target with 'mode: production' must " + advice, | ||
| Detail: adviceDetail, |
There was a problem hiding this comment.
@pietern I'd really like to include location information here and in the other diagnostics in this file, but I'm not sure there is any good way to do that without reverting 6ea5306 (which was originally discussed in #1712 (comment))
There was a problem hiding this comment.
Which location do you want to include?
The conditional checks for absence, so the best possible location seems to be the target definition itself?
If the variables v2 proposal from @denik materializes, we can revisit access to the original targets section. The comment I made in that thread still holds now.
There was a problem hiding this comment.
Yeah, I'd just include the target block as seen in 9466bdc. The same would apply to a couple of the other messages in process_target_mode.
So it seems that the location information would be another benefit of decomposing select_target. We may care more about that in a later stage of the current workspace work. I think we covered all the other trade-offs in the other thread.
Variable resolution is a good point though — it seems like we would want to resolve variables for the selected target. And we could preemptively clean up the other targets to avoid confusion.
In any case, I'd be okay to merge this as is for now. With the extra comments, the surprises of select_target are somewhat alleviated. And we can do without location information for now.
lennartkats-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
|
Note that tests currently fail here; I'll do another quick pass over this based on the outcome of the discussion above. |
lennartkats-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
pietern
approved these changes
Feb 3, 2025
pietern
changed the title
mode: production diagnostic outputmode: production diagnostic output
lennartkats-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
lennartkats-db
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
andrewnester
added a commit
that referenced
this pull request
Feb 27, 2025
Notable changes: Starting this version CLI does not load bundle auth information when CLI command is executed inside the bundle directory with explicitly provided via `-p` flag profile. CLI: * Do not load host from bundle for CLI commands when profile flag is used ([#2335](#2335)). * Fixed accessing required path parameters in CLI generation when --json flag ([#2373](#2373)). Bundles: * Provide instructions for testing in the default-python template ([#2355](#2355)). * Remove `run_as` from the built-in templates ([#2044](#2044)). * Change warning about incomplete permissions section into a recommendation ([#2043](#2043)). * Refine `mode: production` diagnostic output ([#2236](#2236)). * Support serverless mode in default-python template (explicit prompt) ([#2377](#2377)). * Set default data_security_mode to "SINGLE_USER" in bundle templates ([#2372](#2372)). * Fixed spark version check for clusters defined in the same bundle ([#2374](#2374)). API Changes: * Added `databricks genie get-message-query-result-by-attachment` command. OpenAPI commit 99f644e72261ef5ecf8d74db20f4b7a1e09723cc (2025-02-11)
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 27, 2025
Notable changes: Starting this version CLI does not load bundle auth information when CLI command is executed inside the bundle directory with explicitly provided via `-p` flag profile. For more details see the related GitHub issue #1358 CLI: * Do not load host from bundle for CLI commands when profile flag is used ([#2335](#2335)). * Fixed accessing required path parameters in CLI generation when --json flag ([#2373](#2373)). Bundles: * Provide instructions for testing in the default-python template ([#2355](#2355)). * Remove `run_as` from the built-in templates ([#2044](#2044)). * Change warning about incomplete permissions section into a recommendation ([#2043](#2043)). * Refine `mode: production` diagnostic output ([#2236](#2236)). * Support serverless mode in default-python template (explicit prompt) ([#2377](#2377)). * Set default data_security_mode to "SINGLE_USER" in bundle templates ([#2372](#2372)). * Fixed spark version check for clusters defined in the same bundle ([#2374](#2374)). API Changes: * Added `databricks genie get-message-query-result-by-attachment` command. OpenAPI commit 99f644e72261ef5ecf8d74db20f4b7a1e09723cc (2025-02-11)
HariGS-DB
pushed a commit
to HariGS-DB/cli
that referenced
this pull request
Feb 28, 2025
Notable changes: Starting this version CLI does not load bundle auth information when CLI command is executed inside the bundle directory with explicitly provided via `-p` flag profile. For more details see the related GitHub issue databricks#1358 CLI: * Do not load host from bundle for CLI commands when profile flag is used ([databricks#2335](databricks#2335)). * Fixed accessing required path parameters in CLI generation when --json flag ([databricks#2373](databricks#2373)). Bundles: * Provide instructions for testing in the default-python template ([databricks#2355](databricks#2355)). * Remove `run_as` from the built-in templates ([databricks#2044](databricks#2044)). * Change warning about incomplete permissions section into a recommendation ([databricks#2043](databricks#2043)). * Refine `mode: production` diagnostic output ([databricks#2236](databricks#2236)). * Support serverless mode in default-python template (explicit prompt) ([databricks#2377](databricks#2377)). * Set default data_security_mode to "SINGLE_USER" in bundle templates ([databricks#2372](databricks#2372)). * Fixed spark version check for clusters defined in the same bundle ([databricks#2374](databricks#2374)). API Changes: * Added `databricks genie get-message-query-result-by-attachment` command. OpenAPI commit 99f644e72261ef5ecf8d74db20f4b7a1e09723cc (2025-02-11)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
This refines the
mode: productiondiagnostic output now that theDetailproperty is rendered as output. This is a follow-up to #1712.