Re: [PR] refactor(services/http): Move services http out as a crate [opendal]
0lai0 commented on PR #6870: URL: https://github.com/apache/opendal/pull/6870#issuecomment-3673704812 Apologies for missing the message. Thanks @Xuanwo and @koushiro for making the changes. -- 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]
Re: [PR] refactor(services/http): Move services http out as a crate [opendal]
koushiro merged PR #6870: URL: https://github.com/apache/opendal/pull/6870 -- 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]
Re: [PR] refactor(services/http): Move services http out as a crate [opendal]
koushiro commented on PR #6870: URL: https://github.com/apache/opendal/pull/6870#issuecomment-3664710528 Hi @0lai0, the immutable index layer has been split into a separate crate and does not depend on the HTTP service, can you resolve the conflicts? -- 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]
Re: [PR] refactor(services/http): Move services http out as a crate [opendal]
Xuanwo commented on PR #6870: URL: https://github.com/apache/opendal/pull/6870#issuecomment-3631616143 > I'm wondering how to resolve conflicts. I could not use web to edit , so I use git fetch origin and git merge origin/main, but there is a lot of upstream. Anyways, you'll need to start a new PR for moving immutable layer out first. -- 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]
Re: [PR] refactor(services/http): Move services http out as a crate [opendal]
Xuanwo commented on code in PR #6870:
URL: https://github.com/apache/opendal/pull/6870#discussion_r2602118863
##
core/Cargo.toml:
##
@@ -153,6 +153,7 @@ required-features = ["tests"]
[dependencies]
opendal-core = { path = "core", version = "0.55.0", default-features = false }
Review Comment:
No, that's just a question, not related to your PR itself.
--
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]
Re: [PR] refactor(services/http): Move services http out as a crate [opendal]
0lai0 commented on PR #6870: URL: https://github.com/apache/opendal/pull/6870#issuecomment-3631202527 I'm wondering how to resolve conflicts. I could not use web to edit , so I use git fetch origin and git merge origin/main, but there is a lot of upstream. -- 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]
Re: [PR] refactor(services/http): Move services http out as a crate [opendal]
0lai0 commented on code in PR #6870:
URL: https://github.com/apache/opendal/pull/6870#discussion_r2601521979
##
core/core/src/raw/oio/write/api.rs:
##
@@ -43,7 +43,10 @@ pub trait Write: Unpin + Send + Sync {
impl Write for () {
async fn write(&mut self, _: Buffer) -> Result<()> {
-unimplemented!("write is required to be implemented for oio::Write")
+Err(Error::new(
Review Comment:
Sure, thanks for head up.
--
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]
Re: [PR] refactor(services/http): Move services http out as a crate [opendal]
0lai0 commented on code in PR #6870:
URL: https://github.com/apache/opendal/pull/6870#discussion_r2601516940
##
core/Cargo.toml:
##
@@ -153,6 +153,7 @@ required-features = ["tests"]
[dependencies]
opendal-core = { path = "core", version = "0.55.0", default-features = false }
Review Comment:
Is that mean I need to add version here or another place?
--
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]
Re: [PR] refactor(services/http): Move services http out as a crate [opendal]
Xuanwo commented on code in PR #6870:
URL: https://github.com/apache/opendal/pull/6870#discussion_r2597466291
##
core/Cargo.toml:
##
@@ -153,6 +153,7 @@ required-features = ["tests"]
[dependencies]
opendal-core = { path = "core", version = "0.55.0", default-features = false }
Review Comment:
> @Xuanwo do we need to give version for core if its a relative path ?
Yes, we should add the version. Otherwise this crate can't be published to
crates.io.
--
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]
Re: [PR] refactor(services/http): Move services http out as a crate [opendal]
chitralverma commented on code in PR #6870:
URL: https://github.com/apache/opendal/pull/6870#discussion_r2597345097
##
core/Cargo.toml:
##
@@ -153,6 +153,7 @@ required-features = ["tests"]
[dependencies]
opendal-core = { path = "core", version = "0.55.0", default-features = false }
Review Comment:
@Xuanwo do we need to give version for core if its a relative path ?
--
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]
Re: [PR] refactor(services/http): Move services http out as a crate [opendal]
Xuanwo commented on code in PR #6870:
URL: https://github.com/apache/opendal/pull/6870#discussion_r2596471960
##
core/core/src/layers/immutable_index.rs:
##
@@ -211,7 +211,10 @@ impl oio::List for ImmutableDir {
}
#[cfg(test)]
-#[cfg(feature = "services-http")]
+// Note: services-http feature is now in opendal facade crate, not opendal-core
Review Comment:
This the case we need to avoid. Can you move this layer out first?
##
core/core/src/raw/oio/write/api.rs:
##
@@ -43,7 +43,10 @@ pub trait Write: Unpin + Send + Sync {
impl Write for () {
async fn write(&mut self, _: Buffer) -> Result<()> {
-unimplemented!("write is required to be implemented for oio::Write")
+Err(Error::new(
Review Comment:
We shouldn't touch unrelated 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
