Add path exist, is directory, stat #41282
Conversation
| for (auto&& metadata : | ||
| gcs_file->gcs_client.ListObjects(bucket, gcs::Prefix(object))) { | ||
| if (!metadata) { | ||
| TF_SetStatusFromGCSStatus(metadata.status(), status); | ||
| return; | ||
| } | ||
| // We consider a path exists if there is at least one object whose key | ||
| // contains the path. | ||
| return TF_SetStatus(status, TF_OK, ""); | ||
| } |
There was a problem hiding this comment.
This loop runs at most only one time. Is this intended?
There was a problem hiding this comment.
Yes. As I comment, if there is at least one object satisfies the requirement, the path exists.
There was a problem hiding this comment.
In fact, if path = "/path/to/dir/" and there is an object "path/to/dir/file", we consider that this path exists. gcs::Prefix will return the prefix equal to that path ( i.e object name is path+name )
google-ml-butler
bot
added
kokoro:force-run
|
@mihaimaruseac Here is my test with tensorflow What do you think ? |
|
I think that is a bug in |
|
I think no. I am testing with prebuilt tensorflow 2.2.0 so there is nothing releated to |
|
Oh, it's a preexisting bug. I would expect that |
|
So I would like to ask you which value do you expect in the returned result in this case ? I think it should be |
|
In general we should preserve the invariant from above as otherwise there would be subtle to debug issues, even security related ones. In this particular case when we have both a file and a directory with the same name, we can given precedence to directory (as it usually contains more data) and not allowing creation of files/directories if the other one exists (from TF, outside TF user that creates both is responsible for the mess) |
@mihaimaruseac
As I said, this PR can not mimic the current implementation because
google-cloud-cppis missing one feature. I am working on it googleapis/google-cloud-cpp#4494. We will have to wait but it won't take too much time. After that PR is merged, I will do a refactor with thegcscode