-
Notifications
You must be signed in to change notification settings - Fork 323
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
Treat data links even more like symlinks, allow crossing using /
#11926
base: develop
Are you sure you want to change the base?
Conversation
…x faster for data links cases)
This reverts commit 7668476.
Running extra tests as this affects Cloud and S3:
|
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.
Thanks. The builtin changes seem to be done in a "future proof way".
is_directory_builtin file = @Builtin_Method "File.is_directory_builtin" | ||
|
||
is_regular_file_builtin file = @Builtin_Method "File.is_regular_file_builtin" |
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.
Very nice list of builtins. Thank you!
@@ -1,3 +1,50 @@ | |||
private |
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.
And they are all in a private
module. Great.
under a given directory key. This method, however, will only return true if | ||
an object with the exact key exists and is not just an empty marker (as used | ||
sometimes to mark directories). | ||
exists_entity_direct (f : S3_File) -> Boolean = |
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.
This means a user cannot store an object that happens to be zero length, and then verify that it exists. Perhaps we should just treat directory markers are existing objects. I realize there's no good solution to this. But this approach means that if a user creates objects and checks for their existence, they have to know that they cannot do it with one that happens to be empty.
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.
This check is only used in the is_data_link
logic, only if the object's name ends with .datalink
. It is needed to distinguish a datalink file (which if not malformed, should never be empty) from a directory marker.
This check is not done on non-data link entities at all, the exists
method is using a different implementation for regular files/directories. Perhaps I should add a comment to the method that it is only a helper for the datalink heuristic.
Pull Request Description
Important Notes
Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Scala,
Java,
TypeScript,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
or the Snowflake database integration, a run of the Extra Tests has been scheduled.