Fix issue 48 and 60 related to the WorkflowDependencySensor #61
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…an be used to specify the api token
…er secret managers than Databricks Secrets
…rom this version onwards. Removed offset way of pagination and used next_page_token as offset is decpricated from version 2.1 onwards.
…onds since epoch. Also removed expand_tasks in API call as it's not necessary and clutters the response.
…tor needs django which isn't used
BrendBraeckmans
requested review from
asingamaneni,
stikkireddy,
newfront and
a team
as code owners
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #61 +/- ##
==========================================
+ Coverage 88.44% 88.49% +0.04%
==========================================
Files 22 22
Lines 3176 3189 +13
==========================================
+ Hits 2809 2822 +13
Misses 367 367 ☔ View full report in Codecov by Sentry. |
| @@ -5,7 +5,7 @@ fmt: | |||
| @poetry run black . | |||
|
|
|||
| check: black-check mypy | |||
| @poetry run prospector --profile prospector.yaml | |||
| @poetry run prospector --profile prospector.yaml --no-autodetect | |||
There was a problem hiding this comment.
We have this for Spark-Expectations as well. This is good!
asingamaneni
approved these changes
Nov 10, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR contains following changes
--no-autodetectin make check of Makefile cause otherwise prospector needs django which isn't used. I needed this to getmake checkrunning on my machine but not sure what you guys think of thisRelated Issue
Motivation and Context
At this point in time the WorkflowDependencySensor doesn't work at all which blocks multiple users of brickflow.
I know that in the future this functionality will not be needed anymore in brickflow as it's become Databricks native so can be achieved through asset bundles. This however needs development at brickflow side so this PR fixes the issues we face till this is done.
How Has This Been Tested?
I build my own wheel based on this PR and tested it extensively on a workflow making use of the WorkflowDependencySensor
Screenshots (if appropriate):
Types of changes
Due to the change in how the token gets handled in
WorkflowDependencySensorusers will need to adapt their code when they upgrade. I however think there are no users of this feature at the moment as it's not working right now.Checklist: