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

Add versioning for bundle templates #972

Merged
merged 2 commits into from
Nov 30, 2023
Merged

Add versioning for bundle templates #972

merged 2 commits into from
Nov 30, 2023

Conversation

shreyas-goenka
Copy link
Contributor

Changes

This PR adds versioning for bundle templates. Right now there's only logic for the maximum version of templates supported. At some point in the future if we make a breaking template change we can also include a minimum version of template supported by the CLI.

Tests

Unit tests.

@shreyas-goenka
Copy link
Contributor Author

Example UX:

shreyas.goenka@THW32HFW6T mlops-stack % cli bundle init ~/mlops-stack
Error: template schema version 2 is not supported by this version of the CLI. Please upgrade your CLI to the latest version


// Version of the schema. This is used to determine if the schema is
// compatible with the current CLI version.
Version *int `json:"version,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we use MinDatabricksCliVersion for this? They seem to be serving the same purpose

Copy link
Contributor Author

@shreyas-goenka shreyas-goenka Nov 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. We don't need this right now. Longer term though this will become necessary since we need a well defined contract for template features we support in every CLI version.

Minimum CLI version does not have a upper bound on supported CLI version for a template. That does not allows us to make breaking changes to templates in future CLI versions.

The main motivation is to keep templates stable. As mlops-stacks goes GA they will start versioning their releases and at that point we need this to define the minimum and maximum template versions supported by a CLI.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But can it be done with let's say having MaxDatabricksCliVersion later to allow for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, but that's not a great UX. Template authors should not have to track CLI versions supported by their template. A better contract is for them to subscribe to a template version.

I agree though in retrospect that maybe we could have done this in the first place and not added MinDatabricksCliVersion

@shreyas-goenka
Copy link
Contributor Author

Note: The minimum template version supported by the CLI is implied to be 0 (ie all versions are supported). When we decide to not support older versions of templates, we can add that piece of validation in.

@shreyas-goenka shreyas-goenka added this pull request to the merge queue Nov 30, 2023
Merged via the queue into main with commit 1f1ed6d Nov 30, 2023
4 checks passed
@shreyas-goenka shreyas-goenka deleted the schema-version branch November 30, 2023 14:35
andrewnester added a commit that referenced this pull request Dec 6, 2023
CLI:
 * Add documentation for positional args in commands generated from the Databricks OpenAPI specification ([#1033](#1033)).
 * Ask for host when .databrickscfg doesn't exist ([#1041](#1041)).
 * Add list of supported values for flags that represent an enum field ([#1036](#1036)).

Bundles:
 * Fix panic when bundle auth resolution fails ([#1002](#1002)).
 * Add versioning for bundle templates ([#972](#972)).
 * Add support for conditional prompting in bundle init ([#971](#971)).
 * Pass parameters to task when run with `--python-params` and `python_wheel_wrapper` is true ([#1037](#1037)).
 * Change default_python template to auto-update version on each wheel build ([#1034](#1034)).

Internal:
 * Rewrite the friendly log handler ([#1038](#1038)).
 * Move bundle schema update to an internal module ([#1012](#1012)).

Dependency updates:
 * Bump github.com/databricks/databricks-sdk-go from 0.26.0 to 0.26.1 ([#1040](#1040)).
@andrewnester andrewnester mentioned this pull request Dec 6, 2023
github-merge-queue bot pushed a commit that referenced this pull request Dec 6, 2023
CLI:
* Add documentation for positional args in commands generated from the
Databricks OpenAPI specification
([#1033](#1033)).
* Ask for host when .databrickscfg doesn't exist
([#1041](#1041)).
* Add list of supported values for flags that represent an enum field
([#1036](#1036)).

Bundles:
* Fix panic when bundle auth resolution fails
([#1002](#1002)).
* Add versioning for bundle templates
([#972](#972)).
* Add support for conditional prompting in bundle init
([#971](#971)).
* Pass parameters to task when run with `--python-params` and
`python_wheel_wrapper` is true
([#1037](#1037)).
* Change default_python template to auto-update version on each wheel
build ([#1034](#1034)).

Internal:
* Rewrite the friendly log handler
([#1038](#1038)).
* Move bundle schema update to an internal module
([#1012](#1012)).


Dependency updates:
* Bump github.com/databricks/databricks-sdk-go from 0.26.0 to 0.26.1
([#1040](#1040)).
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.

None yet

2 participants