feat(bindings/python): Enhance Reader and Writer by chitralverma · Pull Request #6086 · apache/opendal

chitralverma · 2025-04-24T13:38:49Z

Which issue does this PR close?

Closes #5943

What changes are included in this PR?

Support all read and write options on sync/ async operator open(...)
Support all read options on sync/ async operator read(...)
Support all write options on sync/ async operator write(...)
Add missing capabilities to py bindings
Update documentation
Update all capabilities
Minor fixes linting, typing and typos

Are there any user-facing changes?

No breaking changes, but users can use the new features.

chitralverma · 2025-04-28T15:18:16Z

@Xuanwo @messense @Zheaoli please have a look whenever you get a chance

the 2 failing tests, seem to show a different service specific bug. the if_not_exist capability is there but the behaviour is not correct.

chitralverma · 2025-05-03T09:04:27Z

@Xuanwo @messense @Zheaoli any suggestions you have for this PR?
I think addition of reader/ writer options to AsyncOperator and the lazy streaming copy of data from 1 file to another can be quite useful features.

chitralverma · 2025-05-12T04:43:10Z

@messense @Xuanwo added some benchmarks and updated the scope of this PR.

See Comment

Please let me know if you have some review comments

bindings/python/python/opendal/__init__.pyi

bindings/python/src/options.rs

bindings/python/tests/test_read.py

chitralverma · 2025-05-26T04:36:27Z

@Xuanwo Since #6215 and #6216 have been merged, I think most of the options for reader_with/ writer_with are already covered there. Some one also suggested looking into write_from at a later time. so do you recommend closing this PR or is some here still valid?

Xuanwo · 2025-05-26T05:03:51Z

@Xuanwo Since #6215 and #6216 have been merged, I think most of the options for reader_with/ writer_with are already covered there. Some one also suggested looking into write_from at a later time. so do you recommend closing this PR or is some here still valid?

Hi, I think the additional options for WriteOptions and ReadOptions are still valid. We can go ahead and merge them. However, there are still some concerns regarding the new API related to write_from; perhaps we can leave them to another PR.

erickguan

Great feature and nice chore work!

I added a few suggestions.

bindings/python/pyproject.toml

bindings/python/python/opendal/__init__.pyi

chitralverma · 2025-05-31T22:16:31Z

added a temp fix for the breaking tests.
See discussion #6235

erickguan

Minor comments on test exceptions.

LGTM

erickguan · 2025-06-02T17:54:09Z

bindings/python/tests/test_write.py

+    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)


minor: blank around the keyword as.

Perhaps you can import ConditionNotMatch which aligns with other code. Then:

Suggested change

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)

with pytest.raises(ConditionNotMatch):

async with await async_operator.open(filename, "wb", if_not_exists=True) as w:

w.write(content)

related discussion: #6235

@erickguan this is what i did before, but there was a different issue.
I'll address that in a separate PR.

@chitralverma Thank you so much for bringing up the issue, investigation, and fix! You can add a TODO or a link to the GitHub issue. I will not make this comment in that case :)

erickguan · 2025-06-02T17:55:34Z

bindings/python/tests/test_write.py

+    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)


Suggested change

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)

with pytest.raises(ConditionNotMatch):

with operator.open(filename, "wb", if_not_exists=True) as w:

w.write(content)

@erickguan this is what i did before, but there was a different issue.
I'll address that in a separate PR.

related discussion: #6235

bindings/python/src/options.rs

bindings/python/pyproject.toml

chitralverma · 2025-06-03T06:42:30Z

@Xuanwo @erickguan all your suggestions have been included now, and I've updated the description for this PR.
Please let me know if there's anything else pending before merging this.

Xuanwo · 2025-06-03T06:42:46Z

Thank you @chitralverma for working on this. Although we haven't addressed #6235 yet, I think it's fine to use a workaround in the tests for now.

Will merge after CI passed.

chitralverma requested review from Zheaoli and messense as code owners April 24, 2025 13:38

chitralverma changed the title ~~WIP: feat(bindings/python): Enhance Reader and Writer~~ feat(bindings/python): Enhance Reader and Writer Apr 28, 2025

messense mentioned this pull request May 3, 2025

new feature: Streaming Reader and Writer interfaces in python binding #5943

Closed

1 task

Xuanwo reviewed May 14, 2025

View reviewed changes

bindings/python/python/opendal/__init__.pyi Outdated Show resolved Hide resolved

bindings/python/src/options.rs Outdated Show resolved Hide resolved

bindings/python/tests/test_read.py Outdated Show resolved Hide resolved

chitralverma added 6 commits May 26, 2025 13:25

add reader and writer options to async open

1f790dc

Update test_write.py

2cfb6ad

Add test_async_writer_options test stub

b409cfa

Enhance capability struct with additional conditional operations

b059afa

Implement AsyncFile::write_from

4f37238

revert write_from

e532bbf

chitralverma force-pushed the options-for-rw branch from 85446eb to e532bbf Compare May 26, 2025 08:18

dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels May 26, 2025

chitralverma added 3 commits May 26, 2025 13:51

Merge branch 'apache:main' into options-for-rw

17a44cd

update as per options api

96d854b

Merge branch 'main' into options-for-rw

f5284ff

chitralverma requested review from Xuanwo, erickguan and shanipribadi May 28, 2025 11:31

revert Cargo.lock

d8c8906

dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels May 30, 2025

Reset __base.pyi

f559ddb

dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels May 30, 2025

Merge branch 'main' into options-for-rw

8860387

erickguan approved these changes May 30, 2025

View reviewed changes

chitralverma added 3 commits May 31, 2025 01:03

fix docs for sync/ async read

044f7a4

fix docs for sync/ async open

43203d2

fix docs for sync/ async write

433a2c3

dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels May 30, 2025

temp fix for ConditionNotMatch test

90ca323

chitralverma requested review from Xuanwo and erickguan May 31, 2025 22:20

erickguan approved these changes Jun 2, 2025

View reviewed changes

chitralverma added 7 commits June 3, 2025 11:04

set concurrent to default

e6732be

update capabilities

165b63f

Merge branch 'main' into options-for-rw

da57ef9

add remaining read options

b1c02ab

fix formatting

9351cf2

updated docstrings

af6c562

Merge branch 'main' into options-for-rw

477b7af

Xuanwo merged commit 86d1b40 into apache:main Jun 3, 2025
63 checks passed

chitralverma deleted the options-for-rw branch June 3, 2025 06:51

Xuanwo mentioned this pull request Jun 8, 2025

Added methods and test cases for OpendalBufferedFile fsspec/opendalfs#27

Open

umangbudhwar mentioned this pull request Jun 9, 2025

Enable use of Operator.read(path, offset, size) for efficient range fetches in OpendalBufferedFile fsspec/opendalfs#28

Open

Comments

Conversation

chitralverma commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

What changes are included in this PR?

Are there any user-facing changes?

Uh oh!

chitralverma commented Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chitralverma commented May 3, 2025

Uh oh!

chitralverma commented May 12, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chitralverma commented May 26, 2025

Uh oh!

Xuanwo commented May 26, 2025

Uh oh!

erickguan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chitralverma commented May 31, 2025

Uh oh!

erickguan left a comment

Choose a reason for hiding this comment

Uh oh!

erickguan Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

Xuanwo Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

chitralverma Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

erickguan Jun 4, 2025

Choose a reason for hiding this comment

Uh oh!

erickguan Jun 2, 2025

Choose a reason for hiding this comment

Uh oh!

chitralverma Jun 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chitralverma commented Jun 3, 2025

Uh oh!

Xuanwo commented Jun 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

chitralverma commented Apr 24, 2025 •

edited

Loading

chitralverma commented Apr 28, 2025 •

edited

Loading

Xuanwo commented Jun 3, 2025 •

edited

Loading