You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "caicancai (via GitHub)" <gi...@apache.org> on 2023/11/07 14:42:16 UTC
[PR] refactor(service/tikv): add tikvConfig to implement ConfigDeserializer [incubator-opendal]
caicancai opened a new pull request, #3512:
URL: https://github.com/apache/incubator-opendal/pull/3512
(no comment)
--
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] refactor(service/tikv): Add TikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3512:
URL: https://github.com/apache/incubator-opendal/pull/3512#discussion_r1385961916
##########
core/src/services/tikv/backend.rs:
##########
@@ -20,22 +20,25 @@ use std::fmt::Debug;
use std::fmt::Formatter;
use async_trait::async_trait;
+use serde::Deserialize;
use tikv_client::Config;
use tikv_client::RawClient;
use tokio::sync::OnceCell;
use crate::raw::adapters::kv;
+use crate::raw::*;
use crate::Builder;
use crate::Capability;
use crate::Error;
use crate::ErrorKind;
use crate::Scheme;
use crate::*;
-/// TiKV backend builder
-#[doc = include_str!("docs.md")]
-#[derive(Clone, Default)]
-pub struct TikvBuilder {
+/// Config for Mysql services support.
+#[derive(Default, Deserialize, Clone)]
+#[serde(default)]
+#[non_exhaustive]
+pub struct TikvConfig {
Review Comment:
Please make all fields `pub`.
##########
core/src/services/mod.rs:
##########
@@ -220,6 +220,8 @@ pub use self::redb::Redb;
mod tikv;
#[cfg(feature = "services-tikv")]
pub use self::tikv::Tikv;
+#[cfg(feature = "services-mysql")]
Review Comment:
typo
--
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] refactor(service/tikv): add tikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai closed pull request #3512: refactor(service/tikv): add tikvConfig to implement ConfigDeserializer
URL: https://github.com/apache/incubator-opendal/pull/3512
--
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] refactor(service/tikv): Add TikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #3512:
URL: https://github.com/apache/incubator-opendal/pull/3512#discussion_r1385985066
##########
core/src/services/tikv/backend.rs:
##########
@@ -20,69 +20,101 @@ use std::fmt::Debug;
use std::fmt::Formatter;
use async_trait::async_trait;
+use serde::Deserialize;
use tikv_client::Config;
use tikv_client::RawClient;
use tokio::sync::OnceCell;
use crate::raw::adapters::kv;
+use crate::raw::*;
use crate::Builder;
use crate::Capability;
use crate::Error;
use crate::ErrorKind;
use crate::Scheme;
use crate::*;
-/// TiKV backend builder
-#[doc = include_str!("docs.md")]
-#[derive(Clone, Default)]
-pub struct TikvBuilder {
+/// Config for Mysql services support.
Review Comment:
typo
--
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] refactor(service/tikv): add tikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai closed pull request #3512: refactor(service/tikv): add tikvConfig to implement ConfigDeserializer
URL: https://github.com/apache/incubator-opendal/pull/3512
--
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] refactor(service/tikv): Add TikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3512:
URL: https://github.com/apache/incubator-opendal/pull/3512#issuecomment-1801159969
cc @Xuanwo
--
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] refactor(service/tikv): add tikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3512:
URL: https://github.com/apache/incubator-opendal/pull/3512#issuecomment-1800977229
> You don't have to close and open the PR constantly. A PR can be converted to a draft. 
Thank you for the reminder, next time I start to take such measures, it has been done so far, and it is time to review
--
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] refactor(service/tikv): Add TikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3512:
URL: https://github.com/apache/incubator-opendal/pull/3512#issuecomment-1801197933
@Xuanwo Hello, I find that this part of feature is inconsistent among different methods of backend. This may be due to different developers. I would like to try to organize this part
--
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] refactor(service/tikv): add tikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai closed pull request #3512: refactor(service/tikv): add tikvConfig to implement ConfigDeserializer
URL: https://github.com/apache/incubator-opendal/pull/3512
--
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] refactor(service/tikv): add tikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #3512:
URL: https://github.com/apache/incubator-opendal/pull/3512#issuecomment-1800997815
> if cargo build is fine, it means that the syntax is fine, things don't seem to be what I thought
Service TiKV is not enabled by default which means `cargo build` doesn't build the code you have changed. You can check your code via `cargo clippy --features services-tikv`.
--
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] refactor(service/tikv): add tikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai closed pull request #3512: refactor(service/tikv): add tikvConfig to implement ConfigDeserializer
URL: https://github.com/apache/incubator-opendal/pull/3512
--
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] refactor(service/tikv): Add TikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3512:
URL: https://github.com/apache/incubator-opendal/pull/3512#discussion_r1385964447
##########
core/src/services/tikv/backend.rs:
##########
@@ -20,22 +20,25 @@ use std::fmt::Debug;
use std::fmt::Formatter;
use async_trait::async_trait;
+use serde::Deserialize;
use tikv_client::Config;
use tikv_client::RawClient;
use tokio::sync::OnceCell;
use crate::raw::adapters::kv;
+use crate::raw::*;
use crate::Builder;
use crate::Capability;
use crate::Error;
use crate::ErrorKind;
use crate::Scheme;
use crate::*;
-/// TiKV backend builder
-#[doc = include_str!("docs.md")]
-#[derive(Clone, Default)]
-pub struct TikvBuilder {
+/// Config for Mysql services support.
+#[derive(Default, Deserialize, Clone)]
+#[serde(default)]
+#[non_exhaustive]
+pub struct TikvConfig {
Review Comment:
#[derive(Default, Deserialize, Clone)]
#[serde(default)]
#[non_exhaustive]
pub struct TikvConfig {
/// network address of the TiKV service.
pub endpoints: Option<Vec<String>>,
/// whether using insecure connection to TiKV
pub insecure: bool,
/// certificate authority file path
pub ca_path: Option<String>,
/// cert path
pub cert_path: Option<String>,
/// key path
pub key_path: Option<String>,
}
this?
--
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] refactor(service/tikv): add tikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3512:
URL: https://github.com/apache/incubator-opendal/pull/3512#issuecomment-1801057895
> > if cargo build is fine, it means that the syntax is fine, things don't seem to be what I thought
>
> Service TiKV is not enabled by default which means doesn't build the code you have changed. You can check your code via .`cargo build``cargo clippy --features services-tikv`
Thank you very much
--
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] refactor(service/tikv): add tikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3512:
URL: https://github.com/apache/incubator-opendal/pull/3512#issuecomment-1800971050
I'm sorry that I've committed so many commits that take up CLI resources, I just started learning Rust, thinking that if cargo build is fine, it means that the syntax is fine, things don't seem to be what I thought, and I can only use CLI to correct my compilation errors
--
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] refactor(service/tikv): add tikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on code in PR #3512:
URL: https://github.com/apache/incubator-opendal/pull/3512#discussion_r1385916757
##########
core/src/services/mod.rs:
##########
@@ -213,7 +213,7 @@ pub use self::redb::Redb;
#[cfg(feature = "services-tikv")]
mod tikv;
#[cfg(feature = "services-tikv")]
-pub use self::tikv::Tikv;
+pub use self::tikv::TikvConfig;
Review Comment:
Thanks for the reminder, I'm still familiar with opendal, and I'm sorry for making such a low-level mistake
--
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] refactor(service/tikv): add tikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "suyanhanx (via GitHub)" <gi...@apache.org>.
suyanhanx commented on code in PR #3512:
URL: https://github.com/apache/incubator-opendal/pull/3512#discussion_r1385912980
##########
core/src/services/mod.rs:
##########
@@ -213,7 +213,7 @@ pub use self::redb::Redb;
#[cfg(feature = "services-tikv")]
mod tikv;
#[cfg(feature = "services-tikv")]
-pub use self::tikv::Tikv;
+pub use self::tikv::TikvConfig;
Review Comment:
You may need to use them both.
--
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] refactor(service/tikv): add tikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "suyanhanx (via GitHub)" <gi...@apache.org>.
suyanhanx commented on PR #3512:
URL: https://github.com/apache/incubator-opendal/pull/3512#issuecomment-1800976192
You don't have to close and open the PR constantly. A PR can be converted to a draft.
--
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] refactor(service/tikv): Add TikvConfig to implement ConfigDeserializer [incubator-opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #3512:
URL: https://github.com/apache/incubator-opendal/pull/3512
--
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