Skip to content

Resolve relative paths when building dependency graph#1593

Closed
ericvergnaud wants to merge 31 commits intomainfrom
walk-sys-path
Closed

Resolve relative paths when building dependency graph#1593
ericvergnaud wants to merge 31 commits intomainfrom
walk-sys-path

Conversation

@ericvergnaud
Copy link
Copy Markdown
Contributor

@ericvergnaud ericvergnaud commented Apr 30, 2024

@nfx can be reviewed but depends on #1560

Changes

Add support for cwd to SysPathProvider
Implement LocalNotebookLoader
Provide tests for local files and folders

Linked issues

#1202
Resolves #1499
Resolves #1287

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
  • added unit tests
  • added integration tests
  • verified on staging environment (screenshot attached)

ericvergnaud and others added 30 commits April 25, 2024 10:06
@ericvergnaud ericvergnaud requested review from a team and HariGS-DB April 30, 2024 17:32
@ericvergnaud ericvergnaud marked this pull request as draft April 30, 2024 17:33
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 30, 2024

Codecov Report

Attention: Patch coverage is 94.19795% with 17 lines in your changes are missing coverage. Please review.

Project coverage is 89.48%. Comparing base (00d2e6e) to head (f28c3c8).
Report is 1 commits behind head on main.

Files Patch % Lines
...ricks/labs/ucx/source_code/dependency_resolvers.py 94.61% 5 Missing and 2 partials ⚠️
...icks/labs/ucx/source_code/dependency_containers.py 78.94% 3 Missing and 1 partial ⚠️
...atabricks/labs/ucx/source_code/dependency_graph.py 89.47% 2 Missing ⚠️
src/databricks/labs/ucx/source_code/files.py 85.71% 1 Missing and 1 partial ⚠️
...abricks/labs/ucx/source_code/dependency_loaders.py 97.43% 1 Missing ⚠️
src/databricks/labs/ucx/source_code/notebook.py 92.30% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1593      +/-   ##
==========================================
+ Coverage   89.35%   89.48%   +0.13%     
==========================================
  Files          80       83       +3     
  Lines       10014    10208     +194     
  Branches     1786     1804      +18     
==========================================
+ Hits         8948     9135     +187     
- Misses        696      700       +4     
- Partials      370      373       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Collaborator

@nfx nfx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved classes around to have more consistency with the rest of the codebase, placement clarity, and avoid cyclic imports. It may make sense to reapply this PR from scratch.

@ericvergnaud
Copy link
Copy Markdown
Contributor Author

replaced by #1608

nfx pushed a commit that referenced this pull request May 3, 2024
## Changes
Add support for cwd to SysPathProvider
Implement LocalNotebookLoader
Provide tests for local files and folders

### Linked issues
#1202
Resolves #1499
Resolves #1287

replaces #1593

---------

Co-authored-by: Eric Vergnaud <[email protected]>
@ericvergnaud ericvergnaud deleted the walk-sys-path branch May 10, 2024 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE]: Implement LocalFileLoader and integration tests [FEATURE]: Convert relative paths to full paths when building notebook dependency graph

2 participants