Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #825 +/- ##
==========================================
+ Coverage 85.39% 85.61% +0.21%
==========================================
Files 40 41 +1
Lines 5031 5212 +181
Branches 921 950 +29
==========================================
+ Hits 4296 4462 +166
- Misses 523 536 +13
- Partials 212 214 +2 ☔ View full report in Codecov by Sentry. |
nfx
requested changes
Jan 22, 2024
a61eb09 to
205fb59
Compare
nfx
requested changes
Jan 23, 2024
| assert len(result_set) == 1 | ||
| assert result_set[0].success == 0 | ||
| match = re.findall(fail_regex, result_set[0].failures) | ||
| assert len(match) == 2 |
Collaborator
There was a problem hiding this comment.
parse JSON and assert on concrete failures, don't rely tests on regexes. And mark this PR ready for review.
Added tests.
nfx
approved these changes
Jan 24, 2024
| crawler = ClustersCrawler(ws, MockBackend(), "ucx") | ||
| result_set = list(crawler.snapshot()) | ||
|
|
||
| assert len(result_set) == 4 |
Collaborator
There was a problem hiding this comment.
Can you do assertions on failure messages received? That way we'll be confident in tests doing what they should
| assert len(result_set) == 1 | ||
| assert result_set[0].success == 0 | ||
| failures = json.loads(result_set[0].failures) | ||
| assert 'unsupported config: spark.databricks.passthrough.enabled' in failures |
Collaborator
There was a problem hiding this comment.
I'd suggest to create some helper function for this, as we'll be doing it a lot
nfx
pushed a commit
that referenced
this pull request
Jan 25, 2024
Fixed and issue introduced with PR #825
nfx
added a commit
that referenced
this pull request
Jan 26, 2024
* Added `databricks labs ucx alias` command to create a view of tables from one schema/catalog in another schema/catalog ([#837](#837)). * Added `databricks labs ucx save-aws-iam-profiles` command to scan instance profiles identify AWS S3 access and save a CSV with permissions ([#817](#817)). * Added total view counts in the assessment dashboard ([#834](#834)). * Cleaned up `assess_jobs` and `assess_clusters` tasks in the `assessment` workflow to improve testing and reduce redundancy.([#825](#825)). * Added documentation for the assessment report ([#806](#806)). * Fixed escaping for SQL object names ([#836](#836)). Dependency updates: * Updated databricks-sdk requirement from ~=0.17.0 to ~=0.18.0 ([#832](#832)).
Merged
nfx
added a commit
that referenced
this pull request
Jan 26, 2024
* Added `databricks labs ucx alias` command to create a view of tables from one schema/catalog in another schema/catalog ([#837](#837)). * Added `databricks labs ucx save-aws-iam-profiles` command to scan instance profiles identify AWS S3 access and save a CSV with permissions ([#817](#817)). * Added total view counts in the assessment dashboard ([#834](#834)). * Cleaned up `assess_jobs` and `assess_clusters` tasks in the `assessment` workflow to improve testing and reduce redundancy.([#825](#825)). * Added documentation for the assessment report ([#806](#806)). * Fixed escaping for SQL object names ([#836](#836)). Dependency updates: * Updated databricks-sdk requirement from ~=0.17.0 to ~=0.18.0 ([#832](#832)).
dmoore247
pushed a commit
that referenced
this pull request
Mar 23, 2024
…nd reduce redundancy. (#825)
dmoore247
pushed a commit
that referenced
this pull request
Mar 23, 2024
Fixed and issue introduced with PR #825
dmoore247
pushed a commit
that referenced
this pull request
Mar 23, 2024
* Added `databricks labs ucx alias` command to create a view of tables from one schema/catalog in another schema/catalog ([#837](#837)). * Added `databricks labs ucx save-aws-iam-profiles` command to scan instance profiles identify AWS S3 access and save a CSV with permissions ([#817](#817)). * Added total view counts in the assessment dashboard ([#834](#834)). * Cleaned up `assess_jobs` and `assess_clusters` tasks in the `assessment` workflow to improve testing and reduce redundancy.([#825](#825)). * Added documentation for the assessment report ([#806](#806)). * Fixed escaping for SQL object names ([#836](#836)). Dependency updates: * Updated databricks-sdk requirement from ~=0.17.0 to ~=0.18.0 ([#832](#832)).
This file contains hidden or 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
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.
Changes
Linked issues
closes #818
Relates to #823
Resolves #..
Functionality
databricks labs ucx .........Tests