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

[Kernel] Cleanup the FileNames.java utility methods #3586

Open
vkorukanti opened this issue Aug 20, 2024 · 1 comment · May be fixed by #3588
Open

[Kernel] Cleanup the FileNames.java utility methods #3586

vkorukanti opened this issue Aug 20, 2024 · 1 comment · May be fixed by #3588
Labels
enhancement New feature or request good first issue Good for newcomers good medium issue Good for those with Delta Lake experience kernel
Milestone

Comments

@vkorukanti
Copy link
Collaborator

Some of the utility methods in FileNames.java take strings and some Paths. For string input, it is either the file name or the file path. It is confusing and can easily cause bugs. Unify all APIs to take Path (we anyway create Path in all places when the utility methods are used)

Also, avoid using finding the / to figure out the file name. This could be risky. Instead rely on the Path.

    final int slashIdx = path.lastIndexOf(Path.SEPARATOR); <-- avoid this.
    final String name = path.substring(slashIdx + 1);
    return Long.parseLong(name.split("\\.")[0]);
@vkorukanti vkorukanti added enhancement New feature or request good first issue Good for newcomers good medium issue Good for those with Delta Lake experience kernel labels Aug 20, 2024
@vkorukanti vkorukanti added this to the 3.3.0 milestone Aug 20, 2024
@tlm365 tlm365 linked a pull request Aug 21, 2024 that will close this issue
5 tasks
@tsafacjo
Copy link

can I pick it ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers good medium issue Good for those with Delta Lake experience kernel
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants