Skip to content

Always set default value for source field for tasks if not explicitly set already - #3359

Merged
anton-107 merged 8 commits into
mainfrom
fix/always-send-task-source
Aug 6, 2025
Merged

Always set default value for source field for tasks if not explicitly set already#3359
anton-107 merged 8 commits into
mainfrom
fix/always-send-task-source

Conversation

@andrewnester

@andrewnesterandrewnester commented Aug 5, 2025

Copy link
Copy Markdown
Contributor

Changes

If the job has git_source set, DABs will set source: GIT to all the tasks where source is not already set
If the job has no git_source set, DABs will set source: WORKSPACE to all the tasks where source is not already set

We only do this for a subset of task types which support source field as per https://docs.databricks.com/api/workspace/jobs/create

Why

Fixes#2561

Tests

Added an acceptance test

@eng-dev-ecosystem-bot

eng-dev-ecosystem-bot commented Aug 5, 2025

Copy link
Copy Markdown
Collaborator

Run: 16773732833

Env✅‌pass🔄‌flaky🙈‌skip
✅‌aws linux309461
🔄‌aws windows3082460
✅‌aws-ucws linux415361
✅‌aws-ucws windows416360
✅‌azure linux309460
🔄‌azure windows3073459
🔄‌azure-ucws linux4143358
🔄‌azure-ucws windows4126357
✅‌gcp linux308462
✅‌gcp windows309461
13 failing tests:
Test Nameaws windowsazure windowsazure-ucws linuxazure-ucws windows
TestAccept✅‌pass🔄‌flaky🔄‌flaky✅‌pass
TestAccept/bundle/deploy/dashboard/nested-folders✅‌pass✅‌pass✅‌pass🔄‌flaky
TestAccept/bundle/deploy/dashboard/simple_syncroot✅‌pass✅‌pass✅‌pass🔄‌flaky
TestAccept/bundle/deploy/secret-scope✅‌pass✅‌pass✅‌pass🔄‌flaky
TestAccept/bundle/deployment/bind/dashboard✅‌pass✅‌pass🔄‌flaky✅‌pass
TestAccept/bundle/templates/default-python/integration_classic✅‌pass🔄‌flaky✅‌pass✅‌pass
TestAccept/bundle/templates/default-python/integration_classic/DATABRICKS_CLI_DEPLOYMENT=direct-exp/UV_PYTHON=3.10✅‌pass✅‌pass✅‌pass🔄‌flaky
TestAccept/bundle/templates/default-python/integration_classic/DATABRICKS_CLI_DEPLOYMENT=direct-exp/UV_PYTHON=3.12✅‌pass🔄‌flaky✅‌pass✅‌pass
TestAccept/bundle/templates/default-python/integration_classic/DATABRICKS_CLI_DEPLOYMENT=terraform/UV_PYTHON=3.10✅‌pass✅‌pass✅‌pass🔄‌flaky
TestAccept/bundle/templates/default-python/integration_classic/DATABRICKS_CLI_DEPLOYMENT=terraform/UV_PYTHON=3.12✅‌pass✅‌pass✅‌pass🔄‌flaky
TestFetchRepositoryInfoAPI_FromRepo✅‌pass✅‌pass🔄‌flaky✅‌pass
TestFilerWorkspaceNotebook🔄‌flaky✅‌pass✅‌pass✅‌pass
TestFilerWorkspaceNotebook/sqlNb.sql🔄‌flaky✅‌pass✅‌pass✅‌pass

@denik

denik commented Aug 5, 2025

Copy link
Copy Markdown
Contributor

If the job has get_source set, DABs will set source: GIT to all the tasks where source is not already set

Did you mean "if the job has git_source set"?

@andrewnester

Copy link
Copy Markdown
ContributorAuthor

I meant git_source, fixed the typo

task_key: test_task_foreach_workspace
notebook_task:
notebook_path: ./notebook.py
source: WORKSPACE

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

question -- should we also test "source: GIT" here? Can job/tasks mix different sources?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

It can, hence me adding source:Workspace here. But what would setting source: GIT will give for this job if it's default value anyway anyway?

We can't have source: GIT on non Git jobs so that's why no tests for this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I missed the fact there is job-level git_source setting.

Testing both implicit default and explicit value even if equal to default could be useful but not required.

We can't have source: GIT on non Git jobs so that's why no tests for this

This might be out of scope of this PR but testing wrong configuration to record error message is useful to test assumptions and ensure errors look good.

dyn.Key("resources"),
dyn.Key("jobs"),
dyn.AnyKey(),
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

general question - should this variables be globals? They carry no state and can be initialized once.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I don't think there's a practical difference since this mutator is called only once anyway but I have no concern with making it global either

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For context, mutators can be called multiple times - for static and "python" resources.

task_key: test_task_foreach_workspace
notebook_task:
notebook_path: ./notebook.py
source: WORKSPACE

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I missed the fact there is job-level git_source setting.

Testing both implicit default and explicit value even if equal to default could be useful but not required.

We can't have source: GIT on non Git jobs so that's why no tests for this

This might be out of scope of this PR but testing wrong configuration to record error message is useful to test assumptions and ensure errors look good.


func setSourceIfNotSet(task dyn.Value, defaultSource string) (dyn.Value, error) {
for _, taskType := range supportedTypeTasks {
t, err := dyn.Get(task, taskType)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Question, not a request - Why prefer dyn value over struct here?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

No particular reason, but I prefer to make mutative operations on dynamic config and non-mutative ones on static ones, so that there's a clear distinction between using both

@andrewnester
andrewnester added this pull request to the merge queueAug 6, 2025
@github-merge-queue
github-merge-queueBot removed this pull request from the merge queue due to failed status checks Aug 6, 2025
@anton-107
anton-107 added this pull request to the merge queueAug 6, 2025
Merged via the queue into main with commit 2414730Aug 6, 2025
13 checks passed
@anton-107
anton-107 deleted the fix/always-send-task-source branch August 6, 2025 10:53
deco-sdk-taggingBot added a commit that referenced this pull request Aug 6, 2025
## Release v0.263.0
### CLI
* Show "DEFAULT" as the default profile for `databricks auth login` [#3252](#3252)
### Bundles
* Always set default value for source field for tasks if not explicitly set already ([#3359](#3359))
* Fix panic in auto completion handler for bundler run/open. Regression introduced in 0.259.0. [#3358](#3358)
* Fix error propagation in the `bundle generate dashboard` command. Regression introduced in 0.259.0. [#3354](#3354)
alyssa-db pushed a commit that referenced this pull request Aug 7, 2025
… set already (#3359)
## Changes
If the job has `git_source` set, DABs will set `source: GIT` to all the
tasks where source is not already set
If the job has no `git_source` set, DABs will set `source: WORKSPACE` to
all the tasks where source is not already set
We only do this for a subset of task types which support source field as
per https://docs.databricks.com/api/workspace/jobs/create
## Why
Fixes#2561
## Tests
Added an acceptance test
<!-- If your PR needs to be included in the release notes for next
release,
add a separate entry in NEXT_CHANGELOG.md as part of your PR. -->
alyssa-db pushed a commit that referenced this pull request Aug 7, 2025
## Release v0.263.0
### CLI
* Show "DEFAULT" as the default profile for `databricks auth login` [#3252](#3252)
### Bundles
* Always set default value for source field for tasks if not explicitly set already ([#3359](#3359))
* Fix panic in auto completion handler for bundler run/open. Regression introduced in 0.259.0. [#3358](#3358)
* Fix error propagation in the `bundle generate dashboard` command. Regression introduced in 0.259.0. [#3354](#3354)
denik pushed a commit that referenced this pull request May 20, 2026
… set already (#3359)
## Changes
If the job has `git_source` set, DABs will set `source: GIT` to all the
tasks where source is not already set
If the job has no `git_source` set, DABs will set `source: WORKSPACE` to
all the tasks where source is not already set
We only do this for a subset of task types which support source field as
per https://docs.databricks.com/api/workspace/jobs/create
## Why
Fixes#2561
## Tests
Added an acceptance test
<!-- If your PR needs to be included in the release notes for next
release,
add a separate entry in NEXT_CHANGELOG.md as part of your PR. -->
denik pushed a commit that referenced this pull request May 20, 2026
## Release v0.263.0
### CLI
* Show "DEFAULT" as the default profile for `databricks auth login` [#3252](#3252)
### Bundles
* Always set default value for source field for tasks if not explicitly set already ([#3359](#3359))
* Fix panic in auto completion handler for bundler run/open. Regression introduced in 0.259.0. [#3358](#3358)
* Fix error propagation in the `bundle generate dashboard` command. Regression introduced in 0.259.0. [#3354](#3354)
Sign up for freeto 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.

Error when deploying imported job that moves from source: GIT to source: Workspace

4 participants

@andrewnester@eng-dev-ecosystem-bot@denik@anton-107