Skip to content
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

Simplify error management when building dependency graph #1645

Closed
wants to merge 28 commits into from

Conversation

ericvergnaud
Copy link
Contributor

@ericvergnaud ericvergnaud commented May 6, 2024

Changes

Use MaybeGraph and MaybeDependency structures to simultaneously convey entities and problems

Linked issues

#1202
#1468 is a pre-requisite
Resolves #1559

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)

Eric Vergnaud and others added 27 commits May 2, 2024 15:53
# Conflicts:
#	src/databricks/labs/ucx/contexts/application.py
#	src/databricks/labs/ucx/contexts/workspace_cli.py
# Conflicts:
#	src/databricks/labs/ucx/source_code/files.py
#	src/databricks/labs/ucx/source_code/graph.py
#	src/databricks/labs/ucx/source_code/notebooks/cells.py
#	src/databricks/labs/ucx/source_code/notebooks/sources.py
#	src/databricks/labs/ucx/source_code/path_lookup.py
#	tests/unit/source_code/test_path_lookup.py
#	tests/unit/source_code/test_path_lookup_simulation.py
@ericvergnaud ericvergnaud requested review from a team and william-conti May 6, 2024 20:11
@ericvergnaud
Copy link
Contributor Author

@nfx this PR is based on #1633

# TODO find a solution to enable a different cwd per job/task (maybe it's not necessary or possible?)
return PathLookup.from_sys_path(Path.cwd())
return SysPathLookup.from_sys_path(Path.cwd())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return SysPathLookup.from_sys_path(Path.cwd())
return PathLookup.from_sys_path(Path.cwd())

We use it for both notebooks and files, so it's not quite "sys.path"

@nfx
Copy link
Collaborator

nfx commented May 7, 2024

functionality is on main, we can close this PR

@ericvergnaud ericvergnaud deleted the maybe-graph branch May 10, 2024 11:54
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]: Simplify problem collection during building of dependency graph
2 participants