Xuanwo commented on code in PR #3801:
URL:
https://github.com/apache/incubator-opendal/pull/3801#discussion_r1434832310
##########
core/src/services/gdrive/core.rs:
##########
@@ -314,31 +318,29 @@ impl GdriveCore {
) -> Result<Response<IncomingAsyncBody>> {
let file_id = self.get_file_id_by_path(path).await;
- // when list over a no exist dir, `get_file_id_by_path` will return a
NotFound Error, we should return a empty list in this case.
+ // when list over a no exist dir, we should return a empty list in
this case.
let q = match file_id {
- Ok(file_id) => {
+ Ok(Some(file_id)) => {
format!("'{}' in parents and trashed = false", file_id)
}
- Err(e) => match e.kind() {
- ErrorKind::NotFound => {
- return Response::builder()
- .status(StatusCode::OK)
- .body(IncomingAsyncBody::new(
- Box::new(oio::into_stream(stream::empty())),
- Some(0),
- ))
- .map_err(|e| {
- Error::new(
- ErrorKind::Unexpected,
- &format!("failed to create a empty response
for list: {}", e),
- )
- .set_source(e)
- });
- }
- _ => {
- return Err(e);
- }
- },
+ Ok(None) => {
+ return Response::builder()
Review Comment:
Construct an empty resposne doesn't make me feel good. Do you think it's a
good idea to check `get_file_id_by_path` outside? So `gdrive_list`'s API will
be:
```rust
pub async fn gdrive_list(
&self,
file_id: &str,
page_size: i32,
next_page_token: &str,
) -> Result<Response<IncomingAsyncBody>> {
```
--
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]
