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

Format default-python template #2110

Merged
merged 2 commits into from
Jan 15, 2025

Conversation

kanterov
Copy link
Contributor

@kanterov kanterov commented Jan 9, 2025

Changes

Format code in default-python template, so it's already pre-formatted.

Tests

$ databricks bundle init libs/template/templates/default-python
$ ruff format --diff my_project     
6 files already formatted

Copy link
Contributor

@shreyas-goenka shreyas-goenka left a comment

Choose a reason for hiding this comment

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

Thanks!

@kanterov
Copy link
Contributor Author

kanterov commented Jan 9, 2025

Looks like the same problem exists in libs/template/templates/default-python/template/{{.project_name}}/scratch/exploration.ipynb.tmpl, not sure why I haven't caught it, possibly it isn't included by default

version={{.project_name}}.__version__ + "+" + datetime.datetime.utcnow().strftime("%Y%m%d.%H%M%S"),
version={{.project_name}}.__version__
+ "+"
+ datetime.datetime.utcnow().strftime("%Y%m%d.%H%M%S"),
Copy link
Contributor

Choose a reason for hiding this comment

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

could you increase line width? This should fit in one line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't fit with the default formatting settings, that I believe the majority of developers use. We can extract the version suffix into a separate variable to avoid breaking the line. WDYT?

Copy link

An authorized user can trigger integration tests manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 2110
  • Commit SHA: 6f4ba743510f4eecaff9b8885e30fa4272306246

Checks will be approved automatically on success.

@kanterov
Copy link
Contributor Author

@denik Updated using a new variable, PTAL

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.

@denik @kanterov We should also have test coverage for this and other templates. Formatting will regress without it.

@denik
Copy link
Contributor

denik commented Jan 10, 2025

@pietern @kanterov I agree, we should add 'ruff' to CI and 'make ruff'.

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.

An attempt to ensure the builtin templates are formatted in #2146.

@pietern
Copy link
Contributor

pietern commented Jan 15, 2025

Integration test failure is unrelated. Merging.

@pietern pietern merged commit 25f8ee8 into databricks:main Jan 15, 2025
8 of 9 checks passed
andrewnester added a commit that referenced this pull request Jan 16, 2025
New feature announcement.

You can now manage Databricks Apps using DABs by defining an `app` resource in your bundle configuration.
For more information see Databricks documentation https://docs.databricks.com/en/dev-tools/bundles/resources.html#app

CLI:
 * Filter out system clusters in cluster picker ([#2131](#2131)).
 * Process all the fields in top level request object even if it contains request body ([#2155](#2155)).

Bundles:
 * Added support for Databricks Apps in DABs ([#1928](#1928)).
 * Allow artifact path to be located outside the sync root ([#2128](#2128)).
 * Retry app deployment if there is an active deployment in progress ([#2153](#2153)).
 * Resolve variables in a loop ([#2164](#2164)).
 * Improve resolution of complex variables within complex variables ([#2157](#2157)).
 * Added output message to warn about slower deployments with apps ([#2161](#2161)).
 * Patch references to UC schemas to capture dependencies automatically ([#1989](#1989)).
 * Format default-python template ([#2110](#2110)).
 * Encourage the use of root_path in production to ensure single deployment ([#1712](#1712)).
 * Log warnings to stderr for "bundle validate -o json" ([#2109](#2109)).

Internal:
 * Move merge fix-ups after variable resolution ([#2125](#2125)).
 * Enable linter 'unconvert' and fix the issues found ([#2136](#2136)).
 * Coverage for acceptance tests ([#2123](#2123)).
 * Add acceptance tests for builtin templates ([#2135](#2135)).
 * Add a unique schema for recreate pipeline test ([#2159](#2159)).
 * Migrate resolution tests to acceptance tests ([#2143](#2143)).
 * Update runner for the publish-winget job ([#2105](#2105)).
 * Add a test for complex variable resolution with 3 levels ([#2163](#2163)).

API Changes:
 * Changed `databricks account federation-policy update` command with new required argument order.
 * Changed `databricks account service-principal-federation-policy update` command with new required argument order.

OpenAPI commit 779817ed8d63031f5ea761fbd25ee84f38feec0d (2025-01-08)
Dependency updates:
 * Upgrade TF provider to 1.63.0 ([#2162](#2162)).
 * Bump golangci-lint version to v1.63.4 from v1.63.1 ([#2114](#2114)).
 * Bump astral-sh/setup-uv from 4 to 5 ([#2116](#2116)).
 * Bump golang.org/x/oauth2 from 0.24.0 to 0.25.0 ([#2080](#2080)).
 * Bump github.com/hashicorp/hc-install from 0.9.0 to 0.9.1 ([#2079](#2079)).
 * Bump golang.org/x/term from 0.27.0 to 0.28.0 ([#2078](#2078)).
 * Bump github.com/databricks/databricks-sdk-go from 0.54.0 to 0.55.0 ([#2126](#2126)).
github-merge-queue bot pushed a commit that referenced this pull request Jan 16, 2025
### New feature announcement

#### Databricks Apps support

You can now manage Databricks Apps using DABs by defining an `app`
resource in your bundle configuration.
For more information see Databricks documentation
https://docs.databricks.com/en/dev-tools/bundles/resources.html#app

#### Referencing complex variables in complex variables

You can now reference complex variables within other complex variables.
For more details see #2157

CLI:
* Filter out system clusters in cluster picker
([#2131](#2131)).
* Add command line flags for fields that are not in the API request body
([#2155](#2155)).

Bundles:
* Added support for Databricks Apps in DABs
([#1928](#1928)).
* Allow artifact path to be located outside the sync root
([#2128](#2128)).
* Retry app deployment if there is an active deployment in progress
([#2153](#2153)).
* Resolve variables in a loop
([#2164](#2164)).
* Improve resolution of complex variables within complex variables
([#2157](#2157)).
* Added output message to warn about slower deployments with apps
([#2161](#2161)).
* Patch references to UC schemas to capture dependencies automatically
([#1989](#1989)).
* Format default-python template
([#2110](#2110)).
* Encourage the use of root_path in production to ensure single
deployment ([#1712](#1712)).
* Log warnings to stderr for "bundle validate -o json"
([#2109](#2109)).

API Changes:
* Changed `databricks account federation-policy update` command with new
required argument order.
* Changed `databricks account service-principal-federation-policy
update` command with new required argument order.

OpenAPI commit 779817ed8d63031f5ea761fbd25ee84f38feec0d (2025-01-08)
Dependency updates:
* Upgrade TF provider to 1.63.0
([#2162](#2162)).
* Bump golangci-lint version to v1.63.4 from v1.63.1
([#2114](#2114)).
* Bump astral-sh/setup-uv from 4 to 5
([#2116](#2116)).
* Bump golang.org/x/oauth2 from 0.24.0 to 0.25.0
([#2080](#2080)).
* Bump github.com/hashicorp/hc-install from 0.9.0 to 0.9.1
([#2079](#2079)).
* Bump golang.org/x/term from 0.27.0 to 0.28.0
([#2078](#2078)).
* Bump github.com/databricks/databricks-sdk-go from 0.54.0 to 0.55.0
([#2126](#2126)).

---------

Co-authored-by: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com>
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