-
Notifications
You must be signed in to change notification settings - Fork 1
Adopt FileLike for more pipeline APIs #394
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
Conversation
|
hi @labkey-jeckels, do you anticipate there is going to come a point where FileLike is enforced across modules? It seems like a reasonable enough concept, but as you probably know the sequence module (and related code) is pretty heavy on File-based code. I'm trying to understand where I need to plan a chunk of time to deal with this or not. |
|
We want to move towards using FileLike in all of our server-side APIs. It's a bit of a mess right now with various combinations of File/Path/FileLike. I don't expect we'll ever ban modules from supporting only files, converting from FileLike to File when they like. |
| public FileLike getDataDirectoryFileLike() | ||
| { | ||
| return new File(System.getProperty("java.io.tmpdir")); | ||
| return FileSystemLike.wrapFile(new File(System.getProperty("java.io.tmpdir"))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, all these usages of System.getProperty("java.io.tmpdir") should be replaced with some call to FileUtil. For the TODO list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean something like FileUtil.getTmpDirFileLike(), correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, unless the code actually depends on using this temp directory vs whatever path LabKey has chosen as its default (might be the same, I didn't dive in).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two comments:
- In general, I dont see any real problem with LabKey enforcing a layer over the filesystem, and it's probably an overdue and good idea. In experimented with migrating code in 25.7, and found the FileUtil helpers associated with FileLike to be kinda cumbersome; however, it seems like you're doing a lot on this front in the current release.
- I cannot think of any reason our modules specifically need java.io.tmpdir. That's just the standard way, to my knowledge, for java code to find the tempdir. Having LabKey introduce a layer over this is probably a good idea.
Rationale
Adopt FileLike for more pipeline APIs
Related Pull Requests
Changes
FileLike