Xuanwo commented on code in PR #5184:
URL: https://github.com/apache/opendal/pull/5184#discussion_r1802591927
##########
core/src/raw/http_util/client.rs:
##########
@@ -78,6 +90,41 @@ impl HttpClient {
/// Fetch a request in async way.
pub async fn fetch(&self, req: Request<Buffer>) ->
Result<Response<HttpBody>> {
+ self.fetcher.fetch(req).await
+ }
+}
+
+pub trait HttpFetch: Send + Sync + Unpin + 'static {
Review Comment:
Please add comments to this trait. This trait is exposed to users, and we
expect that users should never interact with `HttpFetchDyn`.
##########
core/src/raw/http_util/client.rs:
##########
@@ -19,22 +19,36 @@ use std::fmt::Debug;
use std::fmt::Formatter;
use std::future;
use std::mem;
+use std::ops::Deref;
use std::str::FromStr;
+use std::sync::Arc;
+use futures::Future;
use futures::TryStreamExt;
use http::Request;
use http::Response;
+use once_cell::sync::Lazy;
use raw::oio::Read;
use super::parse_content_encoding;
use super::parse_content_length;
use super::HttpBody;
+use crate::raw::*;
use crate::*;
+/// Http client used across opendal for loading credentials.
+/// This is merely a temporary solution because reqsign requires a reqwest
client to be passed.
+/// We will remove it after the next major version of reqsign, which will
enable users to provide their own client.
+#[allow(dead_code)]
+pub(crate) static GLOBAL_REQWEST_CLIENT: Lazy<reqwest::Client> =
Lazy::new(reqwest::Client::new);
+
+/// HttpFetcher is a type erased [`HttpFetch`].
+pub type HttpFetcher = Box<dyn HttpFetchDyn>;
Review Comment:
We can use `Arc<dyn HttpFetchDyn>` here to eliminate the need for an extra
`Box`.
##########
core/src/raw/http_util/client.rs:
##########
@@ -78,6 +90,41 @@ impl HttpClient {
/// Fetch a request in async way.
pub async fn fetch(&self, req: Request<Buffer>) ->
Result<Response<HttpBody>> {
+ self.fetcher.fetch(req).await
+ }
+}
+
+pub trait HttpFetch: Send + Sync + Unpin + 'static {
+ /// Fetch a request in async way.
+ fn fetch(
+ &self,
+ req: Request<Buffer>,
+ ) -> impl Future<Output = Result<Response<HttpBody>>> + MaybeSend;
+}
+
+/// HttpFetchDyn is the dyn version of [`HttpFetch`]
+/// which make it possible to use as `Box<dyn HttpFetchDyn>`
+pub trait HttpFetchDyn: Send + Sync + Unpin + 'static {
+ /// The dyn version of [`HttpFetch::fetch`].
+ ///
+ /// This function returns a boxed future to make it object safe.
+ fn fetch_dyn(&self, req: Request<Buffer>) ->
BoxedFuture<Result<Response<HttpBody>>>;
+}
+
+impl<T: HttpFetch + ?Sized> HttpFetchDyn for T {
+ fn fetch_dyn(&self, req: Request<Buffer>) ->
BoxedFuture<Result<Response<HttpBody>>> {
+ Box::pin(self.fetch(req))
+ }
+}
+
+impl<T: HttpFetchDyn + ?Sized> HttpFetch for Box<T> {
Review Comment:
Implement for `Arc<T>` instead.
##########
core/src/raw/http_util/mod.rs:
##########
@@ -25,6 +25,10 @@
mod client;
pub use client::HttpClient;
+/// temporary client used by several features
+#[allow(unused_imports)]
+pub(crate) use client::GLOBAL_REQWEST_CLIENT;
Review Comment:
Please make `HttpFetch` public.
--
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]
