erickguan commented on code in PR #6086:
URL: https://github.com/apache/opendal/pull/6086#discussion_r2121811745
##########
bindings/python/tests/test_write.py:
##########
@@ -176,6 +176,24 @@ async def test_async_writer(service_name, operator,
async_operator):
await async_operator.stat(filename)
+@pytest.mark.asyncio
+@pytest.mark.need_capability("write", "delete", "write_with_if_not_exists")
+async def test_async_writer_options(service_name, operator, async_operator):
+ size = randint(1, 1024)
+ filename = f"test_file_{str(uuid4())}.txt"
+ content = os.urandom(size)
+ f = await async_operator.open(filename, "wb")
+ written_bytes = await f.write(content)
+ assert written_bytes == size
+ await f.close()
+
+ with pytest.raises(Exception)as excinfo:
+ async with await async_operator.open(filename, "wb",
if_not_exists=True) as w:
+ w.write(content)
+ assert "ConditionNotMatch" in str(excinfo.value)
Review Comment:
minor: blank around the keyword `as`.
Perhaps you can import `ConditionNotMatch` which aligns with other code.
Then:
```suggestion
with pytest.raises(ConditionNotMatch):
async with await async_operator.open(filename, "wb",
if_not_exists=True) as w:
w.write(content)
```
##########
bindings/python/src/options.rs:
##########
@@ -20,27 +20,94 @@ use opendal as ocore;
use pyo3::pyclass;
use std::collections::HashMap;
+use std::ops::Bound as RangeBound;
+
+#[pyclass(module = "opendal")]
+#[derive(FromPyObject, Default)]
+pub struct ReadOptions {
+ pub version: Option<String>,
+ pub concurrent: Option<usize>,
+ pub chunk: Option<usize>,
+ pub gap: Option<usize>,
+ pub offset: Option<usize>,
+ pub size: Option<usize>,
+ pub if_match: Option<String>,
+ pub if_none_match: Option<String>,
+}
+
+impl ReadOptions {
+ pub fn make_range(&self) -> (RangeBound<u64>, RangeBound<u64>) {
+ let start_bound = self
+ .offset
+ .map_or(RangeBound::Unbounded, |s| RangeBound::Included(s as u64));
+ let end_bound = self
+ .size
+ .map_or(RangeBound::Unbounded, |e| RangeBound::Excluded(e as u64));
+
+ (start_bound, end_bound)
+ }
+}
+
#[pyclass(module = "opendal")]
#[derive(FromPyObject, Default)]
pub struct WriteOptions {
pub append: Option<bool>,
pub chunk: Option<usize>,
+ pub concurrent: Option<usize>,
+ pub cache_control: Option<String>,
pub content_type: Option<String>,
pub content_disposition: Option<String>,
- pub cache_control: Option<String>,
+ pub content_encoding: Option<String>,
+ pub if_match: Option<String>,
+ pub if_none_match: Option<String>,
+ pub if_not_exists: Option<bool>,
pub user_metadata: Option<HashMap<String, String>>,
}
+impl From<ReadOptions> for ocore::options::ReadOptions {
+ fn from(opts: ReadOptions) -> Self {
+ let r = opts.make_range();
Review Comment:
👍
##########
bindings/python/tests/test_write.py:
##########
@@ -188,3 +206,19 @@ def test_sync_writer(service_name, operator,
async_operator):
operator.delete(filename)
with pytest.raises(NotFound):
operator.stat(filename)
+
+
+@pytest.mark.need_capability("write", "delete", "write_with_if_not_exists")
+def test_sync_writer_options(service_name, operator, async_operator):
+ size = randint(1, 1024)
+ filename = f"test_file_{str(uuid4())}.txt"
+ content = os.urandom(size)
+ f = operator.open(filename, "wb")
+ written_bytes = f.write(content)
+ assert written_bytes == size
+ f.close()
+
+ with pytest.raises(Exception)as excinfo:
+ with operator.open(filename, "wb", if_not_exists=True) as w:
+ w.write(content)
+ assert "ConditionNotMatch" in str(excinfo.value)
Review Comment:
```suggestion
with pytest.raises(ConditionNotMatch):
with operator.open(filename, "wb", if_not_exists=True) as w:
w.write(content)
```
##########
bindings/python/src/options.rs:
##########
@@ -20,27 +20,94 @@ use opendal as ocore;
use pyo3::pyclass;
use std::collections::HashMap;
+use std::ops::Bound as RangeBound;
+
+#[pyclass(module = "opendal")]
+#[derive(FromPyObject, Default)]
+pub struct ReadOptions {
+ pub version: Option<String>,
+ pub concurrent: Option<usize>,
+ pub chunk: Option<usize>,
+ pub gap: Option<usize>,
+ pub offset: Option<usize>,
+ pub size: Option<usize>,
+ pub if_match: Option<String>,
+ pub if_none_match: Option<String>,
+}
+
+impl ReadOptions {
+ pub fn make_range(&self) -> (RangeBound<u64>, RangeBound<u64>) {
+ let start_bound = self
+ .offset
+ .map_or(RangeBound::Unbounded, |s| RangeBound::Included(s as u64));
+ let end_bound = self
+ .size
+ .map_or(RangeBound::Unbounded, |e| RangeBound::Excluded(e as u64));
+
+ (start_bound, end_bound)
+ }
+}
+
#[pyclass(module = "opendal")]
#[derive(FromPyObject, Default)]
pub struct WriteOptions {
pub append: Option<bool>,
pub chunk: Option<usize>,
+ pub concurrent: Option<usize>,
+ pub cache_control: Option<String>,
pub content_type: Option<String>,
pub content_disposition: Option<String>,
- pub cache_control: Option<String>,
+ pub content_encoding: Option<String>,
+ pub if_match: Option<String>,
+ pub if_none_match: Option<String>,
+ pub if_not_exists: Option<bool>,
pub user_metadata: Option<HashMap<String, String>>,
}
+impl From<ReadOptions> for ocore::options::ReadOptions {
+ fn from(opts: ReadOptions) -> Self {
+ let r = opts.make_range();
+ Self {
+ range: r.into(),
+ version: opts.version,
+ if_match: opts.if_match,
+ if_none_match: opts.if_none_match,
+ concurrent: opts.concurrent.unwrap_or(1),
+ chunk: opts.chunk,
+ gap: opts.gap,
+ ..Default::default()
+ }
+ }
+}
+
+impl From<ReadOptions> for ocore::options::ReaderOptions {
+ fn from(opts: ReadOptions) -> Self {
+ Self {
+ version: opts.version,
+ if_match: opts.if_match,
+ if_none_match: opts.if_none_match,
+ concurrent: opts.concurrent.unwrap_or(1),
Review Comment:
Technically this is 100% correct. I feel this is better to be 0 because 0
explains more about the default. opendal-core will use 1 when given a
concurrent=0 for sure.
##########
bindings/python/pyproject.toml:
##########
@@ -70,8 +70,22 @@ cache-keys = [
{ file = "../../core/**/*.rs" },
]
+[tool.ruff]
+line-length = 88
Review Comment:
Alright.
--
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-unsubscr...@opendal.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
