Xuanwo commented on code in PR #2256:
URL:
https://github.com/apache/incubator-opendal/pull/2256#discussion_r1195131980
##########
core/src/services/webdav/backend.rs:
##########
@@ -299,17 +302,81 @@ impl Accessor for WebdavBackend {
}
async fn read(&self, path: &str, args: OpRead) -> Result<(RpRead,
Self::Reader)> {
- let resp = self.webdav_get(path, args.range()).await?;
-
- let status = resp.status();
-
- match status {
- StatusCode::OK | StatusCode::PARTIAL_CONTENT => {
- let meta = parse_into_metadata(path, resp.headers())?;
- Ok((RpRead::with_metadata(meta), resp.into_body()))
+ let mut remaining_retry_times = 3;
Review Comment:
This naming is confusing: we are `retry` insteat of `retry`.
##########
core/src/services/webdav/backend.rs:
##########
@@ -299,17 +302,81 @@ impl Accessor for WebdavBackend {
}
async fn read(&self, path: &str, args: OpRead) -> Result<(RpRead,
Self::Reader)> {
- let resp = self.webdav_get(path, args.range()).await?;
-
- let status = resp.status();
-
- match status {
- StatusCode::OK | StatusCode::PARTIAL_CONTENT => {
- let meta = parse_into_metadata(path, resp.headers())?;
- Ok((RpRead::with_metadata(meta), resp.into_body()))
+ let mut remaining_retry_times = 3;
+ // if response indicates that we should redirect
+ // then modify this variable
+ let mut override_endpoint: Option<String> = None;
+ // sometimes path will also changed according to redirect
+ // response
+ let mut override_path: String = path.to_string();
+
+ debug!(
+ "will read with maximum {} times to retry: path={}",
+ &path, remaining_retry_times
+ );
+ while remaining_retry_times > 0 {
+ debug!("remaining retry times: {}", remaining_retry_times);
+ let resp = self
+ .webdav_get(&override_path, args.range(),
override_endpoint.clone())
+ .await?;
+ let status = resp.status();
+ match status {
+ StatusCode::OK | StatusCode::PARTIAL_CONTENT => {
+ let meta = parse_into_metadata(path, resp.headers())?;
+ return Ok((RpRead::with_metadata(meta), resp.into_body()));
+ }
+ StatusCode::FOUND | StatusCode::TEMPORARY_REDIRECT => {
+ // if server returns redirect HTTP status, then redirect it
+ let redirected_url = parse_location(resp.headers())?
Review Comment:
It's better for us to have a `handle_redirect` function in `http_util`
instead which will handle status_code and parse_location:
```rust
async handle_redirect(client: Client, times: usize, resp: Response) ->
Result<Response>
```
This function will make sure that the response is the correct response after
redirect.
##########
.github/workflows/service_test_webdav.yml:
##########
@@ -112,3 +112,38 @@ jobs:
OPENDAL_WEBDAV_ENDPOINT: http://127.0.0.1:8080
OPENDAL_WEBDAV_USERNAME: bar
OPENDAL_WEBDAV_PASSWORD: bar
+
+ nginx_with_redirect:
+ runs-on: ${{ matrix.os }}
+ strategy:
+ matrix:
+ os:
+ - ubuntu-latest
+ steps:
+ - uses: actions/checkout@v3
+ - name: Setup Rust toolchain
+ uses: ./.github/actions/setup
+
+ - name: Install nginx full for dav_ext modules
+ run: sudo apt install nginx-full
+
+ - name: Start nginx
+ shell: bash
+ working-directory: core
+ run: |
+ mkdir -p /tmp/static
+ mkdir -p /var/lib/nginx
+ chmod a+rw /tmp/static/ # make nginx worker has permission to
operate it
Review Comment:
Please use the following style for comment:
```shell
# make nginx worker has permission to operate it
chmod a+rw /tmp/static/
```
--
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]
