-
Notifications
You must be signed in to change notification settings - Fork 386
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
Added initial support for Multi Task Jobs #853
Conversation
@pietern can you take a look at |
Codecov Report
@@ Coverage Diff @@
## master #853 +/- ##
==========================================
+ Coverage 82.99% 83.56% +0.57%
==========================================
Files 92 92
Lines 8215 8241 +26
==========================================
+ Hits 6818 6887 +69
+ Misses 896 855 -41
+ Partials 501 499 -2
|
If would be nice to split into two diffs - for suppressing the diffs, and actual implementation |
@alexott It's in separate commits |
* provider has to be initialized with `use_multitask_jobs = true` * `task` block of `databricks_job` is currently slice, so adding and removing different tasks might cause confusing, but still correct diffs * we may explore `tf:slice_set` mechanics for `task` blocks, though initial testing turned out to be harder to test * `always_running` parameter still has to be tested for API 2.1 compatibility This implements feature #747
Overall, code looks good. But I would add additional validation - check that dependencies are correct in the |
Okay, tried making this gist contains work-in-progress for making a custom set hash function, though i've observed it creating a phantom task. https://gist.github.com/nfx/620721f593ca42447528598294e8ddc1. perhaps we'll return to it later, but this feature is going to release with a requirement to sort tasks by task key. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code looks good, small comments only...
Can we check what API returns if depends_on
is pointing to incorrect task_key
? Is message understandable? If it's vague, like, "job spec error", then it would be useful to do validation during creation
@alexott it's actually pretty readable:
|
Adding task-order-insensitive |
task
block ofdatabricks_job
is currently slice, so adding and removing different tasks might cause confusing, but still correct diffsThis resolves #747
Syntax example: