Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add if_match support #1748

Closed
7 tasks done
Tracked by #1738
Xuanwo opened this issue Mar 24, 2023 · 7 comments
Closed
7 tasks done
Tracked by #1738

Add if_match support #1748

Xuanwo opened this issue Mar 24, 2023 · 7 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@Xuanwo
Copy link
Member

Xuanwo commented Mar 24, 2023

If-Match is part of HTTP standards. And many storage service support it for conditional requests.

To support If-Match for services, we need to:

https://github.com/apache/incubator-opendal/blob/92e0aefe0f7eceae11a6f45c6b8088764c5c780c/core/src/services/obs/core.rs#L96-L98

Tasks

@Xuanwo Xuanwo added good first issue Good for newcomers help wanted Extra attention is needed labels Mar 24, 2023
@tobehardest
Copy link

Or maybe I can try it, can you assign it to me?

@Xuanwo
Copy link
Member Author

Xuanwo commented Mar 31, 2023

I have assigned this issue to you. Have fun!

@Xuanwo
Copy link
Member Author

Xuanwo commented Apr 10, 2023

Hello, @tobehardest. Do you need help with this issue? I'm available to answer any questions you may have.

@STRRL
Copy link
Contributor

STRRL commented Apr 14, 2023

Hi @Xuanwo , I want to take a try. Could you assign it to me?

IIUC, what I need to do is just like OpRead.with_range, append a field like called like if_match in OpRead, then resolve it in different backends.

Another other thing to concern?

One of my other questions is how to write integration tests for this case?

@Xuanwo
Copy link
Member Author

Xuanwo commented Apr 14, 2023

IIUC, what I need to do is just like OpRead.with_range, append a field like called like if_match in OpRead, then resolve it in different backends.

Yes!

One of my other questions is how to write integration tests for this case?

Good question. To write integration test for this feature, we need #1752 first. In strict mode, backend should return NotSupported for unsupported feature. So we can write test by adding a new case with send if_match and ignore the unsupported error.

@Xuanwo Xuanwo assigned STRRL and unassigned STRRL Apr 14, 2023
@STRRL
Copy link
Contributor

STRRL commented Apr 17, 2023

One of my other questions is how to write integration tests for this case?

Good question. To write integration test for this feature, we need #1752 first. In strict mode, backend should return NotSupported for unsupported feature. So we can write test by adding a new case with send if_match and ignore the unsupported error.

OK. Great, Strict Mode would help a lot for different backend services with/without the support of If-Match.

But even without "Strict Mode", I could still write some integration tests, right? I want to know how to write integration tests with for the If-Match implementation backend service. Maybe just write test cases under core/tests/behavior?

@Xuanwo
Copy link
Member Author

Xuanwo commented Apr 17, 2023

I want to know how to write integration tests with for the If-Match implementation backend service.

Sadly, we can't write this test without strict mode. The behavior for services that doesn't support If-Match is unknown.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants