chitralverma commented on code in PR #6720:
URL: https://github.com/apache/opendal/pull/6720#discussion_r2449017742
##########
bindings/python/ruff.toml:
##########
@@ -73,6 +73,6 @@ strict = true
known-first-party = ["opendal"]
[lint.per-file-ignores]
-"*.pyi" = ["PYI021", "ANN003", "RUF100"]
+"*.pyi" = ["PYI021", "ANN003", "RUF100", "ANN401"]
Review Comment:
necessary for the `new`/ `open` on Operator and AsyncOperator which take
**kwargs.
##########
bindings/python/src/file.rs:
##########
@@ -429,15 +574,24 @@ impl AsyncFile {
})
}
- /// Change the stream position to the given byte offset.
- /// offset is interpreted relative to the position indicated by `whence`.
- /// The default value for whence is `SEEK_SET`. Values for `whence` are:
+ /// Change the position of this file to the given byte offset.
///
- /// * `SEEK_SET` or `0` – start of the stream (the default); offset should
be zero or positive
- /// * `SEEK_CUR` or `1` – current stream position; offset may be negative
- /// * `SEEK_END` or `2` – end of the stream; offset is usually negative
+ /// Parameters
+ /// ----------
+ /// pos : int
+ /// The byte offset (position) to set.
+ /// whence : int, optional
+ /// The reference point for the offset.
+ /// 0: start of file (default); 1: current position; 2: end of file.
///
- /// Return the new absolute position.
+ /// Returns
+ /// -------
+ /// coroutine
+ /// An awaitable that returns the current absolute position.
+ #[gen_stub(override_return_type(
+ type_repr="collections.abc.Awaitable[builtins.int]",
+ imports=("collections.abc", "builtins")
+ ))]
Review Comment:
the generated functions are not async on py side, but the return types have
been overridden to `Awaitable` as advised
[here](https://github.com/Jij-Inc/pyo3-stub-gen/issues/190#issuecomment-3417973228).
same has been done on operator side
##########
bindings/python/src/layers.rs:
##########
@@ -24,15 +24,11 @@ pub trait PythonLayer: Send + Sync {
fn layer(&self, op: Operator) -> Operator;
}
-/// Layer
-///
/// Layers are used to intercept the operations on the underlying storage.
Review Comment:
minor wording changes, to de duplicate heading in mkdocs
##########
bindings/python/src/lib.rs:
##########
@@ -60,9 +60,15 @@ fn _opendal(py: Python, m: &Bound<'_, PyModule>) ->
PyResult<()> {
)?;
// Types module
- add_pymodule!(py, m, "types", [Entry, EntryMode, Metadata])?;
+ add_pymodule!(
+ py,
+ m,
+ "types",
+ [Entry, EntryMode, Metadata, PresignedRequest]
+ )?;
- m.add_class::<PresignedRequest>()?;
+ m.add_class::<Operator>()?;
+ m.add_class::<AsyncOperator>()?;
Review Comment:
initially i wanted to have operators in in their own py module, but then
this would require a major breaking change from `from opendal import Operator`
to `from opendal.operator import Operator`.
I tried messing with the `__init__.py` to get around this, but maybe this
can be taken up in another PR.
##########
bindings/python/tests/test_capability.py:
##########
@@ -19,13 +19,13 @@
def test_capability(service_name, operator):
- cap = operator.capability()
+ cap = operator.full_capability()
assert cap is not None
assert cap.read is not None
def test_capability_exception(service_name, operator):
- cap = operator.capability()
+ cap = operator.full_capability()
Review Comment:
see [this
comment](https://github.com/apache/opendal/pull/6720/files#r2448955236) for
explanation.
--
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]