You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "jorgehermo9 (via GitHub)" <gi...@apache.org> on 2025/04/17 09:59:22 UTC

[PR] feat(services/http): Add operation in http context [opendal]

jorgehermo9 opened a new pull request, #6043:
URL: https://github.com/apache/opendal/pull/6043

   Relates to #5774


-- 
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(services/http): Add operation in http context [opendal]

Posted by "jorgehermo9 (via GitHub)" <gi...@apache.org>.
jorgehermo9 commented on code in PR #6043:
URL: https://github.com/apache/opendal/pull/6043#discussion_r2048773335


##########
core/src/services/http/core.rs:
##########
@@ -79,6 +79,8 @@ impl HttpCore {
             req = req.header(header::RANGE, range.to_header());
         }
 
+        let req = req.extension(Operation::Read);

Review Comment:
   I have styling doubts... Should we
   
   ```
   let req = req.extension(Operation::Read);
   ```
   
   or (which I think it looks cleaner) to set the operation when creating the request
   
   ```
   let mut req = Request::get(&url).extension(Operation::Read);
   ```
   
   
   The latter would require to change the few services (I think there are only 3) that were using `let req = req.extension(Operation::Read);` style.
   
   What do you think?



-- 
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(services/http): Add operation in http context [opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on code in PR #6043:
URL: https://github.com/apache/opendal/pull/6043#discussion_r2049908898


##########
core/src/services/http/core.rs:
##########
@@ -79,6 +79,8 @@ impl HttpCore {
             req = req.header(header::RANGE, range.to_header());
         }
 
+        let req = req.extension(Operation::Read);

Review Comment:
   That's fine. I don't think those style issues are worth a change.



-- 
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(services/http): Add operation in http context [opendal]

Posted by "jorgehermo9 (via GitHub)" <gi...@apache.org>.
jorgehermo9 commented on code in PR #6043:
URL: https://github.com/apache/opendal/pull/6043#discussion_r2048773335


##########
core/src/services/http/core.rs:
##########
@@ -79,6 +79,8 @@ impl HttpCore {
             req = req.header(header::RANGE, range.to_header());
         }
 
+        let req = req.extension(Operation::Read);

Review Comment:
   I have styling doubts... Should we
   
   ```
   let req = req.extension(Operation::Read);
   ```
   
   or (which I think it looks cleaner) to set the operation when creating the request
   
   ```
   let mut req = Request::get(&url).extension(Operation::Read);
   ```
   
   
   The latter would require to change the few services (I think there are only 3) that were using `let req = req.extension(Operation::Read);` stlye.
   
   What do you think?



-- 
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(services/http): Add operation in http context [opendal]

Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #6043:
URL: https://github.com/apache/opendal/pull/6043


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