for_each_task checks on databricks bundle validate #4104
Conversation
idelafuente-oc
requested review from
andrewnester,
anton-107,
denik,
pietern and
shreyas-goenka
as code owners
idelafuente-oc
changed the title
|
Hi @idelafuente-oc ! Thanks for the PR! Contributing to Databricks CLI requires you to sign a CLA If that's something you're okay with, please send a request to sign the CLA to dabs-feedback@databricks.com, and we'll take it from there. Thank you! |
Hello @andrewnester! Since the CLA has been completed, is this good to be reviewed? I added a commit with a small change for a few lines that the gh linter was complaining about. Thank you! |
| return b | ||
| } | ||
|
|
||
| func TestForEachTask_InvalidRetryFields(t *testing.T) { |
There was a problem hiding this comment.
The test looks good overall, but generally for testing mutators, we prefer to use acceptance tests (see acceptance folder). Could you change this test to an acceptance test instead?
There was a problem hiding this comment.
Thank you for the feedback. I added the acceptance tests.
Let me know if I show delete the for_each_task_test.go, I removed some of it's content. Went the unit tests route cause I saw there were some other unit tests there already but missed the acceptance part.
|
An authorized user can trigger integration tests manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
2 participants
Closes: #3977
Changes
Adds to the
databricks bundle validatethe ability to detect properties wrongly added to the parent task in for_each_task.Currently it checks for:
Can easily be extended to other configs with it's custom check.
Why
Currently
databricks bundle validatedoesn't detect this kind of issues, only on deployment this error was thrown. I think it's a good idea to add to the validate logic this typos of checks to prevent issues early on.Tests
I have tested this changes by building the wheel locally with the changes implemented, here is the output of a validate run over code with the issue:
I also added the unit tests for this.