Conversation
…from pip command to enable debugging.
…ice principal roles.
…on_schema.routines is not available
…`list_trigger_runs` rather than expecting one dict per run.
…for Dynamic Management Views.
…ames. Add unit tests to verify that whitespace in credential fields and batch processing of pipeline and trigger runs are is correctly handled. Replace union with concat for Pandas Dataframes.
…o bug_resolve_synpase_profiler_errors
…abase, and driver information.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2289 +/- ##
==========================================
- Coverage 66.44% 66.41% -0.03%
==========================================
Files 99 99
Lines 9089 9093 +4
Branches 974 974
==========================================
Hits 6039 6039
- Misses 2874 2878 +4
Partials 176 176 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
✅ 143/143 passed, 5 flaky, 5 skipped, 34m16s total Flaky tests:
Running from acceptance #3944 |
sundarshankar89
left a comment
There was a problem hiding this comment.
Thank you @eri-adepoju for identifying gaps in the extraction, and for the authentication related changes I will use different approach, I can PR that sperately.
| ``` | ||
|
|
||
| The profiler uses Azure SDK's `DefaultAzureCredential` which attempts authentication in this order: | ||
| 1. **Environment Variables** (Service Principal): |
There was a problem hiding this comment.
Good catch, Thanks for this document update.
src/databricks/labs/lakebridge/resources/assessments/synapse/workspace_extract.py
Show resolved
Hide resolved
src/databricks/labs/lakebridge/resources/assessments/synapse/workspace_extract.py
Show resolved
Hide resolved
| def test_zoneinfo_creation_with_stripped_whitespace() -> None: | ||
| """Test that zoneinfo.ZoneInfo works correctly with stripped timezone strings.""" | ||
| # This tests the core behavior that our code relies on | ||
| tz_with_whitespace = ' America/New_York ' |
There was a problem hiding this comment.
Now I see what has happened, I will tackle this differently for now you cna remove .strip() I will ensure the .credentials.yml doesn't have any spaces likes these when stored.
There was a problem hiding this comment.
I dont think these tests are vaild since we removed strip
| from datetime import date | ||
|
|
||
|
|
||
| def test_pipeline_runs_handles_batches_correctly(): |
There was a problem hiding this comment.
Thanks for adding tests, I m adding type hints in this PR, having type hints enabled and then having tests will help our case.
|
|
||
| - Profiler uses the Python version of Azure SDK libraries to extract information about target Synapse Workspace. | ||
| - For making the Azure API calls using Azure SDK you need an Azure Service Principal with the following role assignments. | ||
| - For making the Azure API calls using Azure SDK, the authenticated identity (user or service principal) needs the following role assignments. |
There was a problem hiding this comment.
Same as above - let's remove mention of service principal until support is added in a future PR.
There was a problem hiding this comment.
This is separate from a service principal accessing information schema tables. Service principals can access Synapse workspaces and Azure monitor metrics with the profiler today.
| """ | ||
|
|
||
| @staticmethod | ||
| def list_serverless_routines(pool_name, redact_sql_text: bool = False) -> str: |
…fensive logs and module comments.
12f5776 to
c569709
Compare
|
@eri-adepoju can you fix fmt errors. |
|
@eri-adepoju there is small conflict can resolve those and make it ready for review changes look good to me. |
All done! |
Sundar approved changes in a previous comment.
Changes
What does this PR do?
Relevant implementation details
Linked issues
Resolves #2287
Functionality
databricks labs lakebridge ...Tests