Xuanwo commented on code in PR #2892:
URL:
https://github.com/apache/incubator-opendal/pull/2892#discussion_r1300128912
##########
core/src/services/gdrive/backend.rs:
##########
@@ -221,12 +221,23 @@ impl GdriveBackend {
"application/vnd.google-apps.folder" => EntryMode::DIR,
_ => EntryMode::FILE,
});
- meta =
meta.with_content_length(metadata.size.parse::<u64>().unwrap_or(0));
+ meta = meta.with_content_length(
+ metadata
+ .size
+ .unwrap_or(String::from("0"))
Review Comment:
Although it's okay to implement it like this, we need to keep in mind that
we should respect the server's response. Our implementation should align with
server's correct behavior.
For this place:
> When will server return a metadata without `size`?
There are possible reasons:
- Gdrive doesn't return size for folder: we should make it clear by set
content length to `0` when we meet a folder.
- Gdrive only return size when we query it: It's a bug, we should fix it.
- Gdrive should return `0` but not: We should return an error instead
because it's violate service API Specs.
- Gdrive will omit size when it's `0`: we should use `serde(default)` and
add comments.
- It's just a bug, gdrive always returns size: we should change `GdriveMeta`
directly.
If we write `unwrap_or(String::from("0"))`, we are ignoring the reasons
behind it, which can create maintenance issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
