-
Notifications
You must be signed in to change notification settings - Fork 127
Remove unused JSON progress logging mode #3812
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.
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
Merged
Merged
Conversation
This file contains hidden or 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
In-place mode was designed to update job progress in place using ANSI escape codes (e.g., showing 'PENDING' → 'RUNNING' → 'TERMINATED' on the same line). However, acceptance tests show jobs typically output only a single state transition: 'Run URL: <url>' followed by '[TIMESTAMP] "job-name" TERMINATED', suggesting the job completes before multiple states can be observed. The default mode selection logic required log-file to not be stderr AND stderr to be a terminal to enable in-place mode, which is an uncommon configuration. Additionally, only JobProgressEvent supported in-place updates while all other events (URLs, errors, pipeline events) fell back to append mode, making the feature inconsistent. The implementation added complexity with ANSI escape codes, terminal detection, and an IsInplaceSupported() interface method across all event types. Since the feature provided minimal practical value and likely was rarely (if ever) enabled by default, it has been removed in favor of the simpler append mode.
The JSON mode for progress logging was designed to output structured JSON events for machine parsing. However, this mode had several limitations: it prevented interactive prompts (Ask/AskSelect methods would error), required --auto-approve for destroy commands, and added complexity with JSON marshaling in the logger. The feature provided minimal practical value as most CLI usage is interactive, and the default append mode is sufficient for both human and machine consumption. Since the mode added unnecessary complexity without clear benefits, it has been removed in favor of the simpler append-only mode.
denik
approved these changes
Oct 24, 2025
Collaborator
40 failing tests:
|
shreyas-goenka
approved these changes
Oct 24, 2025
andrewnester
approved these changes
Oct 24, 2025
Contributor
Author
|
Integration test fails with:
|
pietern
added a commit
that referenced
this pull request
Oct 28, 2025
## Changes This continues the progress logger simplification by migrating from the `Logger.Log()` event-based system to direct cmdio calls. This builds on #3811 and #3812, which made the progress logger effectively only write strings to stderr. ## Why This simplifies the codebase by removing the (unused) event abstraction layer while maintaining functionality. The compatibility layer provides a clean migration path and will eventually be removed once the functionality in those functions have a dedicated new home. ## Tests Tests pass. Analysis of the diff suggests that before/after is functionally equivalent. A minor note is that we no longer have a mutex around `.Log()` calls, but there are no concurrent calls to the function.
deco-sdk-tagging bot
added a commit
that referenced
this pull request
Oct 29, 2025
## Release v0.275.0 ### Notable Changes * Python support for Databricks Asset Bundles is now generally available. ### CLI * Remove `inplace` mode for the `--progress-format` flag ([#3811](#3811)) * Remove `json` mode for the `--progress-format` flag ([#3812](#3812)) * Deprecate the `--progress-format` flag ([#3819](#3819)) ### Bundles * Add support for `--bind` flag in `bundle generate` ([#3782](#3782)) * Add `pydabs` template replacing `experimental-jobs-as-code` template ([#3806](#3806)) * You can now use the top-level `python` section instead of `experimental/python` ([#3540](#3540))
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
The progress logger now only supports
appendmode.Why
JSON mode was added in #276 (March 2023). The intent was to make progress events machine readable but this didn't materialize (we didn't end up using it in the VS Code extension).
This functionality would print events to stderr if the user specified
--progress-format json, or set the an equivalent environment variable. Because the flag is hidden, and there are no online references to the functionality, I believe it is safe to remove. If users take a dependency on JSON output, it should be enabled via the existing--output jsonflag and be written to stdout.After this change is merged, the remaining functionality can be moved into the core
cmdiotypes, and the "progress logger" can be removed. Once there is a single type for all I/O, we have a better shot at improving it.Related change: #3811.
Tests
Tests pass.