You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "tisonkun (via GitHub)" <gi...@apache.org> on 2024/07/20 00:38:16 UTC
[PR] Make config data object [opendal]
tisonkun opened a new pull request, #4915:
URL: https://github.com/apache/opendal/pull/4915
# Which issue does this PR close?
This closes #3197.
# What changes are included in this PR?
See commits message.
# Are there any user-facing changes?
All builder has an associated config data object type now.
--
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(core)!: Make config data object [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #4915:
URL: https://github.com/apache/opendal/pull/4915#issuecomment-2240914571
@Xuanwo Actually this is not a breaking change .. What parts are breaking?
--
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: Make config data object [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4915:
URL: https://github.com/apache/opendal/pull/4915#discussion_r1685230648
##########
core/src/types/builder.rs:
##########
@@ -40,9 +41,18 @@ pub trait Builder: Default {
const SCHEME: Scheme;
/// The accessor that built by this builder.
type Accessor: Access;
+ /// The config for this builder.
+ type Config: DeserializeOwned;
+
+ /// Construct a builder from given config.
+ fn from_config(config: Self::Config) -> Self;
Review Comment:
I want to return `Result<Self>` in `from_config` and `from_map` to avoid panic during the build process. Do you want to implement in this PR or start a new PR for 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] feat(core)!: Make config data object [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #4915:
URL: https://github.com/apache/opendal/pull/4915#issuecomment-2240916552
> @Xuanwo Actually this is not a breaking change .. What parts are breaking?
Builder is a public API. All users implement this API will fail to compile.
--
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: Make config data object [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #4915:
URL: https://github.com/apache/opendal/pull/4915#discussion_r1685230648
##########
core/src/types/builder.rs:
##########
@@ -40,9 +41,18 @@ pub trait Builder: Default {
const SCHEME: Scheme;
/// The accessor that built by this builder.
type Accessor: Access;
+ /// The config for this builder.
+ type Config: DeserializeOwned;
+
+ /// Construct a builder from given config.
+ fn from_config(config: Self::Config) -> Self;
Review Comment:
I want to return `Result<Self>` in `from_config` and `from_map` to avoid panic during the build process. Would you like to help implementing it in this PR or start a new PR for this? Or I can continue the work after this PR.
--
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(core)!: Make config data object [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun merged PR #4915:
URL: https://github.com/apache/opendal/pull/4915
--
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: Make config data object [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #4915:
URL: https://github.com/apache/opendal/pull/4915#issuecomment-2240814785
Seems the root gets override. This is the code difference. Let me look at the test code.
--
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: Make config data object [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #4915:
URL: https://github.com/apache/opendal/pull/4915#issuecomment-2240813746
Or it's the swift backend unstable?
--
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: Make config data object [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #4915:
URL: https://github.com/apache/opendal/pull/4915#issuecomment-2240813600
It seems the swift tests steadily failed. I should not touch the logics around. Perhaps test set up are brittle.
@Xuanwo do you know who write this service integrations?
--
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: Make config data object [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on PR #4915:
URL: https://github.com/apache/opendal/pull/4915#issuecomment-2240820409
Cause found. Fixed.
--
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(core)!: Make config data object [opendal]
Posted by "tisonkun (via GitHub)" <gi...@apache.org>.
tisonkun commented on code in PR #4915:
URL: https://github.com/apache/opendal/pull/4915#discussion_r1685245385
##########
core/src/types/builder.rs:
##########
@@ -40,9 +41,18 @@ pub trait Builder: Default {
const SCHEME: Scheme;
/// The accessor that built by this builder.
type Accessor: Access;
+ /// The config for this builder.
+ type Config: DeserializeOwned;
+
+ /// Construct a builder from given config.
+ fn from_config(config: Self::Config) -> Self;
Review Comment:
Let's make further non-blocker change in a follow-up PR. I can do it, but not in this one unless it's necessary.
--
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