-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Return NotFound for directories in Head and Get (#4230) #4231
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
| let file = File::open(path).map_err(|e| { | ||
| if e.kind() == std::io::ErrorKind::NotFound { | ||
| Error::NotFound { | ||
| let file = match File::open(path).and_then(|f| Ok((f.metadata()?, f))) { |
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 is potentially slightly unfortunate, in that we require an additional syscall, but I couldn't find a mechanism to only open files
43cca27 to
3ff5dda
Compare
object_store/src/azure/client.rs
Outdated
| Err(crate::Error::NotFound { | ||
| path: path.to_string(), | ||
| source: format!( | ||
| "Not a directory, got x-ms-resource-type: {}", |
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.
Should this be, "Not a file"?
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.
Oops lol
Edit: fixed, good spot
* Return NotFound for directories in Head and Get (#4230) * Fix webdav * Fix error message
Which issue does this PR close?
Closes apache/arrow-rs-object-store#165
Rationale for this change
I spent a non-trivial amount of time helping diagnose an issue with apache/datafusion#6183 that boiled down to a head request being made instead of a list request, because the
ListingTableUrldidn't end with a/. Normally this would have returned a 404 NotFound which would have made the issue easy to identify, however, because it was an Azure hierarchical namespace this didn't occur.LocalFileSystem also runs into a similar issue.
What changes are included in this PR?
Are there any user-facing changes?
LocalFileSystem and MicrosoftAzure will no longer return directories for get and head requests