## Changes
1. Add a Why section to the pull request template
2. Slightly improve language in the existing sections
## Why
Providing the right context for the reviewer in the PR description is
important as it usually cannot be inferred from the code itself. The new
section directly prompts the requester to provide such context
## Tests
Checked that the markdown is still rendered correctly in the local
viewer
## Changes
1. remove t.Skip directive from TestAuthDescribeSuccess integration test
2. change the test code to account for environments where host value can
be prefixed with the https protocol part
## Why
This enables integration test that exercises `databricks auth describe`
command, which was previously throwing errors in the CI/CD pipeline
## Tests
Integration test is passing
Ignore output files using gitignore syntax.
## Changes
New Ignore setting in test.toml that will ignore specified files (syntax
is gitignore).
## Why
I'm using it in #2396 to ignore virtual env. It includes a lot of files.
The regular 'rm -fr .venv' approach only works if script get to that
point, but due to errors it might abort early. In that cases test runner
prints all unexpected files, polluting output. Ignoring those files at
test runner level ensure you never see them.
## Tests
Updated selftest/basic.
## Changes
1. Refactored `TestSparkJarTaskDeployAndRunOnVolumes` and
`TestSparkJarTaskDeployAndRunOnWorkspace` to use a table-driven approach
for better organization of similar tests
2. Implemented `testutil.HasJDK()` to replace `testutil.RequireJDK` to
be able to skip tests
3. Ensured the test suite properly fails if no compatible Java version
is found
## Why
It can be tricky to have Java 8 installed on modern dev environments
(e.g. Mac on Apple M3 chip). The absence of which previously caused the
Spark Jar task tests to fail when run locally. This refactoring allows
such environments to be able to run "SparkJar" tests using a newer
Databricks Runtime.
## Tests
1. Ran `TestSparkJarTaskDeployAndRunOnVolumes` and
`TestSparkJarTaskDeployAndRunOnWorkspace` locally on Mac with Java11
installed.
2. Checked that tests against older runtimes are still being run and
passing in CI/CD environments
## Changes
This PR adds a warning which gives users clear guidance when they try to
use variable interpolation for an auth field.
## Tests
Modify existing acceptance test.
## Changes
Defining an include section in config files other than the main
`databricks.yml` file fails silently. With this PR users will get a
warning when they try this.
## Tests
Acceptance test.
## Tests
Manually, I have a test that fails.
Before:
```
=== NAME TestAccept
server.go:195:
----------------------------------------
No stub found for pattern: GET /api/2.1/clusters/get
To stub a response for this request, you can add
the following to test.toml:
[[Server]]
Pattern = "GET /api/2.1/clusters/get"
Response.Body = '''
<response body here>
'''
Response.StatusCode = <response status-code here>
----------------------------------------
```
After:
```
server.go:203: No handler for URL: /api/2.1/clusters/get?cluster_id=0717-132531-5opeqon1
Body: [0 bytes]
For acceptance tests, add this to test.toml:
[[Server]]
Pattern = "GET /api/2.1/clusters/get"
Response.Body = '<response body here>'
# Response.StatusCode = <response code if not 200>
```
## Changes
- Instead of constructing chains of mutators and then executing them,
execute them directly.
- Remove functionality related to chain-building: Seq, If, Defer,
newPhase, logString.
- Phases become functions that apply the changes directly rather than
construct mutator chains that will be called later.
- Add a helper ApplySeq to call multiple mutators, use it where
Apply+Seq were used before.
This is intended to be a refactoring without functional changes, but
there are a few behaviour changes:
- Since defer() is used to call unlock instead of bundle.Defer()
unlocking will now happen even in case of panics.
- In --debug, the phase names are are still logged once before start of
the phase but each entry no longer has 'seq' or phase name in it.
- The message "Deployment complete!" was printed even if
terraform.Apply() mutator had an error. It no longer does that.
## Motivation
The use of the chains was necessary when mutators were returning a list
of other mutators instead of calling them directly. But that has since
been removed, so now the chain machinery have no purpose anymore.
Use of direct functions simplifies the logic and makes bugs more
apparent and easy to fix.
Other improvements that this unlocks:
- Simpler stacktraces/debugging (breakpoints).
- Use of functions with narrowly scoped API: instead of mutators that
receive full bundle config, we can use focused functions that only deal
with sections they care about prepareGitSettings(currentGitSection) ->
updatedGitSection. This makes the data flow more apparent.
- Parallel computations across mutators (within phase): launch
goroutines fetching data from APIs at the beggining, process them once
they are ready.
## Tests
Existing tests.
## Changes
Instead of LocalOnly with non-composable semantics there are two
composable options:
- Local - enable test locally
- Cloud - enable test on the cloud
By default Cloud is switched off except in bundle (but not in
bundle/variables and bundle/help).
## Tests
Using this in #2383 to have test that runs on cloud but not locally.
## Changes
Since at this moment we set default to 'no', interactively it should
also default to 'no'. However, it just uses the first option.
## Tests
Manually running `cli bundle init default-python`
## Changes
- Add 'serverless' prompt to default-python template (default is
currently set to "no").
- This is a simplified version of
https://github.com/databricks/cli/pull/2348 with 'auto' functionality
removed.
## Tests
- Split default-python into default-python/classic,
default-python/serverless, default-python/serverless-customcatalog.
- Manually check that "bundle init default-python" with serverless=yes
can be deployed and run on dogfood and test env.
## Changes
Now when `profile` flag is used we won't pick up host from bundle
anymore and use the one provided by -p flag
Previous behaviour in the context of bundle
```
databricks current-user me -p profile_name
Error: cannot resolve bundle auth configuration: config host mismatch: profile uses host https://non-existing-subdomain.databricks.com, but CLI configured to use https://foo.com
```
New behaviour (make an api call)
```
databricks current-user me -p profile_name
{
email: "foo@bar.com"
...
}
```
We still load bundle configuration when `-t` flag provide because we
want to load host information from the target.
Fixes#1358
## Tests
Added acceptance test
## Changes
1. Change the **default-python** bundle template to set
`data_security_mode` of a cluster to SINGLE_USER
2. Change the **experimental-jobs-as-code** bundle template to set
`data_security_mode` of a cluster to SINGLE_USER
## Why
Explicitly adding this field saves experienced users from confusion onto
what security mode is applied to the cluster
## Tests
Changed existing unit and integration tests to pass with this change
## Changes
This PR:
1. No longer sets the `DATABRICKS_CLI_PARENT_PID` environment variable
since it was never required in the first place and was mistakenly merged
in the inital PR.
2. Performs minor cleanup based on post merge feedback in
https://github.com/databricks/cli/pull/2354.
## Tests
N/A
## Changes
Added PyPi and Maven libraries tests
Needed for this PR since we don't currently have any coverage for PyPi
or Maven libraries
https://github.com/databricks/cli/pull/2382
## Changes
Previously using python wheel tasks in the tasks with compute referering
to interactive cluster defied in the same bundle would produce a warning
like below
```
GET /api/2.1/clusters/get?cluster_id=${resources.clusters.development_cluster.id}
< HTTP/2.0 400 Bad Request
< {
< "error_code": "INVALID_PARAMETER_VALUE",
< "message": "Cluster ${resources.clusters.development_cluster.id} does not exist"
< } pid=14465 mutator=seq mutator=initialize mutator=seq mutator=PythonWrapperWarning sdk=true
```
This PR fixes it by making sure that we check spark version for such
clusters based on its bundle configuration and don't make API calls
## Tests
Added acceptance test
## Changes
CLI generation template was using RequiredPathField from incorrect
request entity (body field from request and not request itself). Thus
for some of the commands required path parameters were not required when
--json was specified.
## Tests
Regenerated commands work correctly
## Changes
I'm not completely sure what is lowercasing it, we can investigate this
later. This unblocks tests on main.
> Name: project_name_kLvfBngEcUEI
> Uploading
project_name_klvfbngecuei-0.0.1+[NUMID].[NUMID]-py3-none-any.whl...
## Tests
`CLOUD_ENV=aws go test --timeout 3h -v -run TestDefaultPython/3.9
./integration/bundle`
## Changes
- Get rid of artifacts.DetectPackages which is a thin wrapper around
artifacts/whl.DetectPackage
- Get rid of parsing name out of setup.py. Do not randomize either, use
a static one.
## Tests
Existing tests.
## Changes
Added missing .gitignore files to templates
## Tests
There were some incorrect snapshots of gitignore files in acceptance
tests, probably generated by testing infra. Updated them to new files
---------
Co-authored-by: Lennart Kats (databricks) <lennart.kats@databricks.com>
## Changes
Previously, one could not set `LocalOnly=true` in parent directory and
then override it with `LocalOnly=false` in child directory. This is
because, `false` is considered empty value by mergo.
In order to distinguish between 'explicitly set to false' and 'not set',
I've changed all simple variables in config to be pointers. Now, one can
always override those, because non-nil pointers are not null (with
mergo.WithoutDereference option).
## Tests
Manually:
```
~/work/cli/acceptance/bundle/templates/default-python % cat test.toml # add this new file
LocalOnly = false
~/work/cli/acceptance/bundle/templates/default-python % CLOUD_ENV=aws go test ../../.. -run ^TestAccept$/^bundle$/^templates$/^default-python$ -v
(the test is no longer skipped)
```
## Changes
- Added some missing descriptions to annotations.yml
- Fixed links in the resource reference doc template
## Tests
<!-- How is this tested? -->
## Changes
- Instead of collecting requests in memory and writing them at the end
of the test, write them right away. Then test authors can do filtering
with jq in 'script' or collect individual files per different command.
- testserver is now simpler - it just calls a caller-provided function.
The logging logic is moved to acceptance_test.go.
See https://github.com/databricks/cli/pull/2359/files#r1967591173
## Tests
Existing tests.
## Changes
Changes the warning about an incomplete / implicit permissions section
into a recommendation, and does a minor bit of cleanup.
## Tests
New unit test.
## Changes
This refines the `mode: production` diagnostic output now that the
`Detail` property is rendered as output. This is a follow-up to
https://github.com/databricks/cli/pull/1712.
## Changes
This removes the `run-as` property from the default templates. It's a
useful property but it still only works for jobs and it makes the
default databricks.yml a bit longer. It seems like users can just learn
about it from the docs and/or vary their deployment identity.
Depends on https://github.com/databricks/cli/pull/1712.
## Changes
This PR adds a library for spawning a daemon process. Our needs are
different from those of a typical daemon process in that we want to
handle being orphaned gracefully. This is because, in the vast majority
of telemetry use cases, the main CLI process (i.e., the parent process)
will exit before the telemetry process has a chance to finish uploading
the logs.
To achieve this we "detach" the child process from the parent process,
which requires different flags for UNIX vs. non-unix systems.
Here are the properties that we want to ensure for our telemetry
child/daemon processes:
1. They do not block the parent process.
Reason: The main CLI process should not be blocked on the telemetry
child process.
2. The child can read from stdin. The parent can write to stdin.
Reason: Telemetry logs will be passed to the child process via stdin.
3. Output logs do not leak from the child process.
Reason: Telemetry logs should not be visible to users of the CLI.
## Tests
Unit test
## Changes
To avoid build warnings and errors in docs build we need to escape
symbols that are treated as syntax elements
## Tests
<!-- How is this tested? -->
## Changes
diff.py is like "diff -r -U2" but it applies replacements first to the
argument.
This allows comparing different output files and directories but ignore
differences that are going to be replaced by placeholders.
This is useful for tests that record large amount of files, specifically
"bundle init" with standard templates. In those tests, changing one
parameter results in a small diff so recording the full directory is not
helpful, because it's hard to see what changed there. I'm using it in
implementation of serverless mode for templates that need it: #2348 The
serverless templates are slightly different from classic, capturing the
diff helps to see exactly where.
Related small changes:
- Add [TESTROOT] replacement for absolute path to acceptance directory
in git repo.
- Add $TESTDIR env var for absolute path to a given test in git repo.
## Tests
- New test acceptance/selftest/diff to test the helper.
- Via #2348 which makes use of this feature.
This is needed for b.WorkspaceClient().CurrentWorkspaceID(ctx) which is
used by initialize_urls.go mutator ("bundle summary") #2316
It also also needed for to call serverless detection endpoint #2348
Builds on top of #2338
This is a bugfix release to address an issue where jobs with tasks with
a
libraries section with PyPI packages could not be deployed.
Bundles:
* Revert changes related to basename check for local libraries
([#2345](https://github.com/databricks/cli/pull/2345)).
## Changes
These changes break the use of non-local libraries (such as PyPI
libraries).
This reverts the set so we can cut a patch release and take a closer
look later.
Original PRs are #2297 and #2341.
Issue reported in #2343.
## Tests
Manually confirmed that a bundle with PyPI package in libraries now
deploys fine.
## Changes
The CheckForSameNameLibraries mutator incorrectly assumed all resource
libraries define libraries as paths of the `string` type, but some
libraries, such as PyPi and Maven, define them as objects.
This PR addresses this issue. It was introduced in #2297.
## Tests
Added regression test.
## Changes
Handlers now receive testserver.Request and return any which could be
- string or []byte (returns it as is but sets content-type to json or
plain text depending on content)
- struct (encodes it as json and sets content-type to json)
- testserver.Response (full control over status and headers)
Note if testserver.Response is returned from the handler, it's Body
attribute can still be an object. In that case, it'll be serialized and
appropriate content-type header will be added.
The config is now using the same testserver.Response struct, the same
logic applies both configured responses and responses returned from
handlers.
As a result, one can set headers both in Golang handlers and in
test.toml.
This also fixes a bug with RecordRequest not seeing the body if it was
already consumed by the handler.
## Tests
- Existing rests.
- acceptance/selftest/server is extended to set response header.
## Changes
Reverse the order of stubs to match expectation (leaf configuration
takes precedence over parent configuration).
Follow up to #2334 .
## Tests
acceptance/selftest/server is extended with duplicate handler
## Changes
This PR adds acceptance tests for native Databricks auth methods: basic,
oauth, and pat.
In the future we could compare this with auth credentials used by
downstream tools like TF or the telemetry process to ensure consistent
auth credentials are picked up and used.
Note:
We do not add acceptance tests for other auth methods like Azure because
they communicate with external endpoints. To test them locally, we would
need to set up a reverse proxy server, which is out of scope for this
change.
## Tests
N/A
## Changes
- Currently if you define [[Server]] block, you disable the default
server implementation. With this change [[Server]] block takes
precedence over default server but default server remains.
- Switched mux implementation to
[gorilla/mux](https://github.com/gorilla/mux) -- unlike built-in it does
not panic if you set two handlers on the same part (instead the earliest
one wins). It also does not have any dependencies.
- Move acceptance/selftest into acceptance/selftest/basic and added
acceptance/selftest/server that demoes server override.
- Rewrite server set up to ensure that env vars and replacements are set
up correctly. Previously replacements for DATABRICKS_HOST referred to
default server, not to the custom server.
- Avoid calling CurrentUser.Me() in the local case. This allows
overriding /api/2.0/preview/scim/v2/Me, which we use in some tests (e.g.
bundle/templates-machinery/helpers-error). Previously the test passed
because CurrentUser.Me() was calling default server which is incorrect
but it happened to make the tests pass.
- The default server is now available on DATABRICKS_DEFAULT_HOST env
var.
- Rewrite "not found" handler in local test to handle error better (do
not raise http500 when header is already written).
## Tests
New acceptance test selftest/server specifically tests that both custom
and default handlers are available in a single test.
Anton Nekipelov
Denis Bilenko
Andrew Nester
shreyas-goenka
dependabot[bot]
Ilya Kuznetsov
Julia Crawford (Databricks)
Lennart Kats (databricks)
Pieter Noordhuis