-
Notifications
You must be signed in to change notification settings - Fork 80
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
Enhance Advice with source location #1447
Enhance Advice with source location #1447
Conversation
code: str | ||
message: str | ||
source_type: str | ||
source_path: str |
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.
we should keep WHAT (e.g. deprecation) problem we have from WHERE (e.g. notebook or a file) that problem is.
@dataclass
class NotebookAdvice:
path: str
advice: list[Advice]
@dataclass
class FileAdvice:
path: str
advice: list[Advice]
that being said, we may need another datastructure to map onto a database table, but it should have Job ID and Job Name as well and most likely live within the assessment
package:
@dataclass
class WorkloadAdvice:
workflow_id: int
workflow_name: str
object_type: string
object_path: string # alternatively, object_id, to make links easier
code: str
message: str
line: int
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.
See my comment in #1448
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1447 +/- ##
==========================================
- Coverage 90.02% 88.29% -1.74%
==========================================
Files 62 77 +15
Lines 7430 9380 +1950
Branches 1335 1658 +323
==========================================
+ Hits 6689 8282 +1593
- Misses 470 731 +261
- Partials 271 367 +96 ☔ View full report in Codecov by Sentry. |
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.
do not modify Advice
.
superseded by #1529 |
…dencies (#1529) ## Changes Create a DependencyProblem class, than can later be converted to Advice Whenever a problem is encountered, ### Linked issues #1202 Resolves #1444 Resolves #1431 Resolves #1439 ### Functionality - [ ] added relevant user documentation - [ ] added new CLI command - [ ] modified existing command: `databricks labs ucx ...` - [ ] added a new workflow - [ ] modified existing workflow: `...` - [ ] added a new table - [ ] modified existing table: `...` ### Tests - [ ] manually tested - [x] added unit tests - [ ] added integration tests - [ ] verified on staging environment (screenshot attached) Supersedes PRs #1447 and #1448
Changes
Enhance Advice with source type and location
Linked issues
Resolves #1444
Functionality
databricks labs ucx ...
...
...
Tests