kylebarron commented on code in PR #540:
URL: 
https://github.com/apache/arrow-rs-object-store/pull/540#discussion_r2543505058


##########
src/client/mod.rs:
##########
@@ -488,46 +492,76 @@ impl ClientOptions {
         self
     }
 
-    /// Sets what protocol is allowed. If `allow_http` is :
-    /// * false (default):  Only HTTPS are allowed
-    /// * true:  HTTP and HTTPS are allowed
+    /// Sets what protocol is allowed.
+    ///
+    /// If `allow_http` is :
+    /// * `false` (default):  Only HTTPS are allowed
+    /// * `true`:  HTTP and HTTPS are allowed
     pub fn with_allow_http(mut self, allow_http: bool) -> Self {
         self.allow_http = allow_http.into();
         self
     }
     /// Allows connections to invalid SSL certificates
-    /// * false (default):  Only valid HTTPS certificates are allowed
-    /// * true:  All HTTPS certificates are allowed
     ///
-    /// # Warning
+    /// If `allow_invalid_certificates` is :
+    /// * `false` (default):  Only valid HTTPS certificates are allowed
+    /// * `true`:  All HTTPS certificates are allowed
+    ///
+    /// <div class="warning">
+    ///
+    /// **Warning**
     ///
     /// You should think very carefully before using this method. If
     /// invalid certificates are trusted, *any* certificate for *any* site
     /// will be trusted for use. This includes expired certificates. This
     /// introduces significant vulnerabilities, and should only be used
     /// as a last resort or for testing
+    ///
+    /// </div>
     pub fn with_allow_invalid_certificates(mut self, allow_insecure: bool) -> 
Self {
         self.allow_insecure = allow_insecure.into();
         self
     }
 
-    /// Only use http1 connections
+    /// Only use HTTP/1 connections
+    ///
+    /// # See Also
+    /// * [`Self::with_http2_only`] if you only want to use HTTP/2
+    /// * [`Self::with_allow_http2`] if you want to use HTTP/1 or HTTP/2
     ///
-    /// This is on by default, since http2 is known to be significantly slower 
than http1.
+    /// <div class="warning">
+    /// This is off by default, since HTTP/2 is known to be significantly 
slower than http1.
+    /// </div>
     pub fn with_http1_only(mut self) -> Self {
         self.http2_only = false.into();
         self.http1_only = true.into();
         self
     }
 
-    /// Only use http2 connections
+    /// Only use HTTP/2 connections
+    ///
+    /// # See Also
+    /// * [`Self::with_http1_only`] if you only want to use HTTP/1
+    /// * [`Self::with_allow_http2`] if you want to use HTTP/1 or HTTP/2
+    ///
+    /// <div class="warning">
+    /// This is off by default, since HTTP/2 is known to be significantly 
slower than HTTP/1.
+    /// </div>
     pub fn with_http2_only(mut self) -> Self {
         self.http1_only = false.into();
         self.http2_only = true.into();
         self
     }
 
-    /// Use http2 if supported, otherwise use http1.
+    /// Use HTTP/2 if supported, otherwise use HTTP/1.
+    ///
+    /// # See Also
+    /// * [`Self::with_http1_only`] if you only want to use HTTP/1
+    /// * [`Self::with_http2_only`] if you only want to use HTTP/2
+    ///
+    /// <div class="warning">
+    /// This is off by default, since HTTP/2 is known to be significantly 
slower than HTTP/1.

Review Comment:
   Also I don't know if this is correct; I usually think of HTTP2 being faster 
than HTTP1 because of multiplexing. But I know there has been some discussion 
somewhere on some issue here on why we don't default to HTTP2



-- 
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]

Reply via email to