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

[TECH DEBT] Improve file handling by FileLoader #3640

Merged
merged 8 commits into from
Feb 5, 2025

Conversation

JCZuurmond
Copy link
Member

Changes

Improve file handling by FileLoader:

  • Verify the FileLoader resolves files using the PathLookup
  • Handle file opening errors, like PermissionError, FileNotFoundError
  • Handle file with BOM encodings
  • Handle empty files

Linked issues

Breaks up #3586

Functionality

  • rewrote code linting related resources

Tests

  • Added tests for FileLoader

@JCZuurmond JCZuurmond added migrate/code Abstract Syntax Trees and other dark magic migrate/python Pull requests that update Python code python Pull requests that update Python code internal this pull request won't appear in release notes labels Feb 5, 2025
@JCZuurmond JCZuurmond self-assigned this Feb 5, 2025
@JCZuurmond JCZuurmond requested a review from a team as a code owner February 5, 2025 10:13
Copy link

github-actions bot commented Feb 5, 2025

✅ 57/57 passed, 2 skipped, 25m17s total

Running from acceptance #8193

):
super().__init__(cwd, sys_paths or [])

self.successfully_resolved_paths = set[Path]() # The paths that were successfully resolved
Copy link
Member Author

Choose a reason for hiding this comment

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

Note that this attribute should be used in tests instead of passing a set for the linted paths as this is correct and it actually was a bug that the paths were not resolved

@JCZuurmond JCZuurmond enabled auto-merge February 5, 2025 11:19
@JCZuurmond JCZuurmond added this pull request to the merge queue Feb 5, 2025
Merged via the queue into main with commit 429319e Feb 5, 2025
7 checks passed
@JCZuurmond JCZuurmond deleted the fix/file-loader-handles-files branch February 5, 2025 11:32
github-merge-queue bot pushed a commit that referenced this pull request Feb 5, 2025
…ing back on its path (#3641)

## Changes

Let `LocalFileLinter` reuse the `Dependency` instead falling back on its
path as this introduce duplicate ways of loading files.

### Linked issues

Progresses #3514
Breaks up #3520
Stacked on:
- [x] #3640
- [x] #3639
- [x] #3638

### Functionality

- [x] modified existing command: `databricks labs ucx
migrate-local-code`

### Tests

- [ ] manually tested
- [x] modified and added unit tests
- [x] modified and added integration tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal this pull request won't appear in release notes migrate/code Abstract Syntax Trees and other dark magic migrate/python Pull requests that update Python code python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants