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

feat: Add write_total_max_size in Capability #3309

Merged
merged 3 commits into from
Oct 26, 2023
Merged

feat: Add write_total_max_size in Capability #3309

merged 3 commits into from
Oct 26, 2023

Conversation

poltao
Copy link
Contributor

@poltao poltao commented Oct 16, 2023

For #3216 , D1 limited the Maximum string, BLOB or table row size to 1M, but gen_bytes() randomly generates 1~4M content. It will result in the following error:

D1Response { result: [], success: false, errors: [D1Error { message: "string or blob too big", code: 7500 }], messages: [] }

Maybe we can support configuring max_size via environment variable?

@Xuanwo
Copy link
Member

Xuanwo commented Oct 16, 2023

I prefer addressing this issue from service capability side.

@poltao
Copy link
Contributor Author

poltao commented Oct 16, 2023

I prefer addressing this issue from service capability side.

I'm not sure I understand what you mean. How do we addressing this issue from service capability side?
And when the randomly generated content is greater than 1m, behavioral tests can fail, don't we need to be concerned?

@Xuanwo
Copy link
Member

Xuanwo commented Oct 16, 2023

I'm not sure I understand what you mean. How do we addressing this issue from service capability side?

I haven't thought about this deeply yet, but perhaps we can consider adding a write_total_max_size to the Capability like this:

https://github.com/apache/incubator-opendal/blob/82f98772363a1548f9184c07f58cc85fcdf857ae/core/src/types/capability.rs#L89-L92

This way, both users and our tests can determine the maximum size supported by this service.

@poltao
Copy link
Contributor Author

poltao commented Oct 25, 2023

@Xuanwo Add new capability attribute write_total_max_size, plz help review.

core/Cargo.toml Outdated Show resolved Hide resolved
core/tests/behavior/utils.rs Outdated Show resolved Hide resolved
@Xuanwo Xuanwo changed the title feat: behavior test support config max_size feat: Add write_total_max_size in Capability Oct 25, 2023
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

@Xuanwo
Copy link
Member

Xuanwo commented Oct 26, 2023

The only task here is to make clippy happy:

error: using `clone` on type `Capability` which implements the `Copy` trait
   --> core/tests/behavior/blocking_write.rs:435:34
    |
435 |     let (content, _) = gen_bytes(op.info().full_capability().clone());
    |                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try removing the `clone` call: `op.info().full_capability()`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#clone_on_copy

@Xuanwo Xuanwo merged commit 3aad395 into apache:main Oct 26, 2023
@poltao poltao deleted the feat/behavior branch October 26, 2023 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants