You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "Xuanwo (via GitHub)" <gi...@apache.org> on 2024/01/17 09:01:22 UTC

[PR] feat(services/gdrive): Use trash instead of permanently deletes [incubator-opendal]

Xuanwo opened a new pull request, #4002:
URL: https://github.com/apache/incubator-opendal/pull/4002

   Fix https://github.com/apache/incubator-opendal/issues/3994


-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(services/gdrive): Use trash instead of permanently deletes [incubator-opendal]

Posted by "Young-Flash (via GitHub)" <gi...@apache.org>.
Young-Flash commented on code in PR #4002:
URL: https://github.com/apache/incubator-opendal/pull/4002#discussion_r1455039597


##########
core/src/services/gdrive/builder.rs:
##########
@@ -23,41 +23,63 @@ use std::sync::Arc;
 use chrono::DateTime;
 use chrono::Utc;
 use log::debug;
+use serde::Deserialize;
 use tokio::sync::Mutex;
 
 use super::backend::GdriveBackend;
-use crate::raw::HttpClient;
 use crate::raw::{normalize_root, PathCacher};
+use crate::raw::{ConfigDeserializer, HttpClient};
 use crate::services::gdrive::core::GdriveSigner;
 use crate::services::gdrive::core::{GdriveCore, GdrivePathQuery};
 use crate::Scheme;
 use crate::*;
 
+/// [GoogleDrive](https://drive.google.com/) configuration.
+#[derive(Default, Deserialize)]
+#[serde(default)]
+#[non_exhaustive]
+pub struct GdriveConfig {
+    /// The root for gdrive
+    pub root: Option<String>,
+    /// Access token for gdrive.
+    pub access_token: Option<String>,
+    /// Refresh token for gdrive.
+    pub refresh_token: Option<String>,
+    /// Client id for gdrive.
+    pub client_id: Option<String>,
+    /// Client secret for gdrive.
+    pub client_secret: Option<String>,

Review Comment:
   `trash` is the default behavior in gdrive



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(services/gdrive): Use trash instead of permanently deletes [incubator-opendal]

Posted by "ClSlaid (via GitHub)" <gi...@apache.org>.
ClSlaid commented on code in PR #4002:
URL: https://github.com/apache/incubator-opendal/pull/4002#discussion_r1455028875


##########
core/src/services/gdrive/builder.rs:
##########
@@ -23,41 +23,63 @@ use std::sync::Arc;
 use chrono::DateTime;
 use chrono::Utc;
 use log::debug;
+use serde::Deserialize;
 use tokio::sync::Mutex;
 
 use super::backend::GdriveBackend;
-use crate::raw::HttpClient;
 use crate::raw::{normalize_root, PathCacher};
+use crate::raw::{ConfigDeserializer, HttpClient};
 use crate::services::gdrive::core::GdriveSigner;
 use crate::services::gdrive::core::{GdriveCore, GdrivePathQuery};
 use crate::Scheme;
 use crate::*;
 
+/// [GoogleDrive](https://drive.google.com/) configuration.
+#[derive(Default, Deserialize)]
+#[serde(default)]
+#[non_exhaustive]
+pub struct GdriveConfig {
+    /// The root for gdrive
+    pub root: Option<String>,
+    /// Access token for gdrive.
+    pub access_token: Option<String>,
+    /// Refresh token for gdrive.
+    pub refresh_token: Option<String>,
+    /// Client id for gdrive.
+    pub client_id: Option<String>,
+    /// Client secret for gdrive.
+    pub client_secret: Option<String>,

Review Comment:
   Maybe add an option for permanent remove or temporary trash by default?
   
   And only judging by config is kind of too dull, we may also extend OpDelete with an optional field, as an override to the config.
   
   



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(services/gdrive): Use trash instead of permanently deletes [incubator-opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #4002:
URL: https://github.com/apache/incubator-opendal/pull/4002


-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(services/gdrive): Use trash instead of permanently deletes [incubator-opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4002:
URL: https://github.com/apache/incubator-opendal/pull/4002#discussion_r1455031851


##########
core/src/services/gdrive/builder.rs:
##########
@@ -23,41 +23,63 @@ use std::sync::Arc;
 use chrono::DateTime;
 use chrono::Utc;
 use log::debug;
+use serde::Deserialize;
 use tokio::sync::Mutex;
 
 use super::backend::GdriveBackend;
-use crate::raw::HttpClient;
 use crate::raw::{normalize_root, PathCacher};
+use crate::raw::{ConfigDeserializer, HttpClient};
 use crate::services::gdrive::core::GdriveSigner;
 use crate::services::gdrive::core::{GdriveCore, GdrivePathQuery};
 use crate::Scheme;
 use crate::*;
 
+/// [GoogleDrive](https://drive.google.com/) configuration.
+#[derive(Default, Deserialize)]
+#[serde(default)]
+#[non_exhaustive]
+pub struct GdriveConfig {
+    /// The root for gdrive
+    pub root: Option<String>,
+    /// Access token for gdrive.
+    pub access_token: Option<String>,
+    /// Refresh token for gdrive.
+    pub refresh_token: Option<String>,
+    /// Client id for gdrive.
+    pub client_id: Option<String>,
+    /// Client secret for gdrive.
+    pub client_secret: Option<String>,

Review Comment:
   > Maybe add an option for permanent remove or temporary trash by default?
   
   We can add an option for permanent deletion if users request it in the future.
   
   > And only judging by config is kind of too dull, we may also extend OpDelete with an optional field, as an override to the config.
   
   I feel like this idea is not cool since not all services have such concept.



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(services/gdrive): Use trash instead of permanently deletes [incubator-opendal]

Posted by "suyanhanx (via GitHub)" <gi...@apache.org>.
suyanhanx commented on PR #4002:
URL: https://github.com/apache/incubator-opendal/pull/4002#issuecomment-1895618542

   Thanks!


-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(services/gdrive): Use trash instead of permanently deletes [incubator-opendal]

Posted by "ClSlaid (via GitHub)" <gi...@apache.org>.
ClSlaid commented on code in PR #4002:
URL: https://github.com/apache/incubator-opendal/pull/4002#discussion_r1455028875


##########
core/src/services/gdrive/builder.rs:
##########
@@ -23,41 +23,63 @@ use std::sync::Arc;
 use chrono::DateTime;
 use chrono::Utc;
 use log::debug;
+use serde::Deserialize;
 use tokio::sync::Mutex;
 
 use super::backend::GdriveBackend;
-use crate::raw::HttpClient;
 use crate::raw::{normalize_root, PathCacher};
+use crate::raw::{ConfigDeserializer, HttpClient};
 use crate::services::gdrive::core::GdriveSigner;
 use crate::services::gdrive::core::{GdriveCore, GdrivePathQuery};
 use crate::Scheme;
 use crate::*;
 
+/// [GoogleDrive](https://drive.google.com/) configuration.
+#[derive(Default, Deserialize)]
+#[serde(default)]
+#[non_exhaustive]
+pub struct GdriveConfig {
+    /// The root for gdrive
+    pub root: Option<String>,
+    /// Access token for gdrive.
+    pub access_token: Option<String>,
+    /// Refresh token for gdrive.
+    pub refresh_token: Option<String>,
+    /// Client id for gdrive.
+    pub client_id: Option<String>,
+    /// Client secret for gdrive.
+    pub client_secret: Option<String>,

Review Comment:
   Maybe add a setting as preference between permanent remove or temporary trash?
   
   And only judging by config is kind of too dull, we may also extend OpDelete with an optional field, as an override to the config.
   
   



-- 
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: commits-unsubscribe@opendal.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org