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.

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

Skip for_each_task when generating the bundle schema #1204

Merged
merged 2 commits into from
Feb 13, 2024
Merged

Conversation

shreyas-goenka
Copy link
Contributor

Changes

Bundle schema generation does not support recursive API fields. This PR skips generation for for_each_task until we add proper support for recursive types in the bundle schema.

Tests

Manually. This fixes the generation of the CLI and the bundle schema command works as expected, with the sub-schema for for_each_task being set to null in the output.

"for_each_task": null,

@codecov-commenter
Copy link

codecov-commenter commented Feb 13, 2024

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (36241ee) 51.91% compared to head (f9c3c07) 51.88%.

Files Patch % Lines
bundle/schema/openapi.go 50.00% 2 Missing and 1 partial ⚠️
bundle/schema/schema.go 40.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1204      +/-   ##
==========================================
- Coverage   51.91%   51.88%   -0.03%     
==========================================
  Files         299      299              
  Lines       16607    16611       +4     
==========================================
- Hits         8621     8619       -2     
- Misses       7357     7361       +4     
- Partials      629      631       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@pietern pietern changed the title Skip for_each_task when generating the bundle schema Skip for_each_task when generating the bundle schema Feb 13, 2024
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.

Please add a backlink to this PR in the code comments for reference.

@shreyas-goenka shreyas-goenka added this pull request to the merge queue Feb 13, 2024
Merged via the queue into main with commit 52b813b Feb 13, 2024
4 checks passed
@shreyas-goenka shreyas-goenka deleted the fix-schema branch February 13, 2024 14:20
andrewnester added a commit that referenced this pull request Feb 15, 2024
CLI:
 * Ignore environment variables for `auth profiles` ([#1189](#1189)).
 * Update LICENSE ([#1013](#1013)).

Bundles:
 * Added `bundle deployment bind` and `unbind` command ([#1131](#1131)).
 * Use allowlist for Git-related fields to include in metadata ([#1187](#1187)).
 * Added `--restart` flag for `bundle run` command ([#1191](#1191)).
 * Generate correct YAML if custom_tags or spark_conf is used for pipeline or job cluster configuration ([#1210](#1210)).

Internal:
 * Move folders package into libs ([#1184](#1184)).
 * Log time it takes for profile to load ([#1186](#1186)).
 * Use mockery to generate mocks compatible with testify/mock ([#1190](#1190)).
 * Retain partially valid structs in `convert.Normalize` ([#1203](#1203)).
 * Skip `for_each_task` when generating the bundle schema ([#1204](#1204)).
 * Regenerate the CLI using the same OpenAPI spec as the SDK ([#1205](#1205)).
 * Avoid race-conditions while executing sub-commands ([#1201](#1201)).

API Changes:
 * Changed `databricks connections delete` command with new required argument order.
 * Changed `databricks connections get` command with new required argument order.
 * Changed `databricks connections update` command with new required argument order.
 * Added `databricks tables exists` command.
 * Changed `databricks volumes delete` command with new required argument order.
 * Changed `databricks volumes read` command with new required argument order.
 * Changed `databricks volumes update` command with new required argument order.
 * Added `databricks lakehouse-monitors` command group.
 * Removed `databricks files get-status` command.
 * Added `databricks files create-directory` command.
 * Added `databricks files delete-directory` command.
 * Added `databricks files get-directory-metadata` command.
 * Added `databricks files get-metadata` command.
 * Added `databricks files list-directory-contents` command.
 * Removed `databricks pipelines reset` command.
 * Changed `databricks account settings delete-personal-compute-setting` command with new required argument order.
 * Removed `databricks account settings read-personal-compute-setting` command.
 * Changed `databricks account settings update-personal-compute-setting` command with new required argument order.
 * Added `databricks account settings get-personal-compute-setting` command.
 * Removed `databricks settings delete-default-workspace-namespace` command.
 * Removed `databricks settings read-default-workspace-namespace` command.
 * Removed `databricks settings update-default-workspace-namespace` command.
 * Added `databricks settings delete-default-namespace-setting` command.
 * Added `databricks settings delete-restrict-workspace-admins-setting` command.
 * Added `databricks settings get-default-namespace-setting` command.
 * Added `databricks settings get-restrict-workspace-admins-setting` command.
 * Added `databricks settings update-default-namespace-setting` command.
 * Added `databricks settings update-restrict-workspace-admins-setting` command.
 * Changed `databricks token-management create-obo-token` command with new required argument order.
 * Changed `databricks token-management get` command to return .
 * Changed `databricks clean-rooms delete` command with new required argument order.
 * Changed `databricks clean-rooms get` command with new required argument order.
 * Changed `databricks clean-rooms update` command with new required argument order.
 * Changed `databricks dashboards create` command . New request type is .
 * Added `databricks dashboards update` command.

OpenAPI commit c40670f5a2055c92cf0a6aac92a5bccebfb80866 (2024-02-14)
Dependency updates:
 * Bump github.com/hashicorp/hc-install from 0.6.2 to 0.6.3 ([#1200](#1200)).
 * Bump golang.org/x/term from 0.16.0 to 0.17.0 ([#1197](#1197)).
 * Bump golang.org/x/oauth2 from 0.16.0 to 0.17.0 ([#1198](#1198)).
 * Bump github.com/databricks/databricks-sdk-go from 0.30.1 to 0.32.0 ([#1199](#1199)).
@andrewnester andrewnester mentioned this pull request Feb 15, 2024
github-merge-queue bot pushed a commit that referenced this pull request Feb 15, 2024
CLI:
* Ignore environment variables for `auth profiles`
([#1189](#1189)).
* Update LICENSE file to match Databricks license language
([#1013](#1013)).

Bundles:
* Added `bundle deployment bind` and `unbind` command
([#1131](#1131)).
* Use allowlist for Git-related fields to include in metadata
([#1187](#1187)).
* Added `--restart` flag for `bundle run` command
([#1191](#1191)).
* Generate correct YAML if `custom_tags` or `spark_conf` is used for
pipeline or job cluster configuration
([#1210](#1210)).

Internal:
* Move folders package into libs
([#1184](#1184)).
* Log time it takes for profile to load
([#1186](#1186)).
* Use mockery to generate mocks compatible with testify/mock
([#1190](#1190)).
* Retain partially valid structs in `convert.Normalize`
([#1203](#1203)).
* Skip `for_each_task` when generating the bundle schema
([#1204](#1204)).
* Regenerate the CLI using the same OpenAPI spec as the SDK
([#1205](#1205)).
* Avoid race-conditions while executing sub-commands
([#1201](#1201)).

API Changes:
 * Added `databricks tables exists` command.
 * Added `databricks lakehouse-monitors` command group.
 * Removed `databricks files get-status` command.
 * Added `databricks files create-directory` command.
 * Added `databricks files delete-directory` command.
 * Added `databricks files get-directory-metadata` command.
 * Added `databricks files get-metadata` command.
 * Added `databricks files list-directory-contents` command.
 * Removed `databricks pipelines reset` command.
* Changed `databricks account settings delete-personal-compute-setting`
command with new required argument order.
* Removed `databricks account settings read-personal-compute-setting`
command.
* Changed `databricks account settings update-personal-compute-setting`
command with new required argument order.
* Added `databricks account settings get-personal-compute-setting`
command.
* Removed `databricks settings delete-default-workspace-namespace`
command.
* Removed `databricks settings read-default-workspace-namespace`
command.
* Removed `databricks settings update-default-workspace-namespace`
command.
 * Added `databricks settings delete-default-namespace-setting` command.
* Added `databricks settings delete-restrict-workspace-admins-setting`
command.
 * Added `databricks settings get-default-namespace-setting` command.
* Added `databricks settings get-restrict-workspace-admins-setting`
command.
 * Added `databricks settings update-default-namespace-setting` command.
* Added `databricks settings update-restrict-workspace-admins-setting`
command.
* Changed `databricks token-management create-obo-token` command with
new required argument order.
 * Changed `databricks token-management get` command to return .
* Changed `databricks dashboards create` command . New request type is .
 * Added `databricks dashboards update` command.

OpenAPI commit c40670f5a2055c92cf0a6aac92a5bccebfb80866 (2024-02-14)
Dependency updates:
* Bump github.com/hashicorp/hc-install from 0.6.2 to 0.6.3
([#1200](#1200)).
* Bump golang.org/x/term from 0.16.0 to 0.17.0
([#1197](#1197)).
* Bump golang.org/x/oauth2 from 0.16.0 to 0.17.0
([#1198](#1198)).
* Bump github.com/databricks/databricks-sdk-go from 0.30.1 to 0.32.0
([#1199](#1199)).

---------

Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>
github-merge-queue bot pushed a commit that referenced this pull request Mar 28, 2024
## Changes
This PR fixes bundle schema being broken because `for_each_task: null`
was set in the generated schema. This is not valid according to the JSON
schema specification and thus the Red Hat YAML VSCode extension was
failing to parse the YAML configuration.

This PR fixes: #1312

## Tests
The fix itself was tested manually. I asserted that the autocompletion
works now. This was mistakenly overlooked the first time around when the
regression was introduced in #1204
because the YAML extension provides best-effort autocomplete suggestions
even if the JSON schema fails to load.

To prevent future regressions we also add a test to assert that the JSON
schema generated itself is a valid JSON schema object. This is done via
using the `ajv-cli` to validate the schema. This package is also used by
the Red Hat YAML extension and thus provides a high fidelity check for
ensuring the JSON schema is valid.

Before, with the old schema:
```
shreyas.goenka@THW32HFW6T cli-versions % ajv validate -s proj/schema-216.json -d ../bundle-playground-3/databricks.yml
schema proj/schema-216.json is invalid
error: schema is invalid: data/properties/resources/properties/jobs/additionalProperties/properties/tasks/items/properties/for_each_task must be object,boolean, data/properties/resources/properties/jobs/additionalProperties/properties/tasks/items must be array, data/properties/resources/properties/jobs/additionalProperties/properties/tasks/items must match a schema in anyOf
```

After, with the new schema:
```
shreyas.goenka@THW32HFW6T cli-versions % ajv validate -s proj/schema-dev.json -d ../bundle-playground-3/databricks.yml
../bundle-playground-3/databricks.yml valid
```

After, autocomplete suggestions:

<img width="600" alt="Screenshot 2024-03-27 at 6 35 57 PM"
src="https://github.com/databricks/cli/assets/88374338/d0a62402-e323-4f36-854d-332b33cbeab8">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants