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

Use buffering=0 for local device #132

Merged
merged 1 commit into from
Sep 18, 2024
Merged

Use buffering=0 for local device #132

merged 1 commit into from
Sep 18, 2024

Conversation

phako
Copy link
Contributor

@phako phako commented Oct 30, 2023

Fixes an issue when opening the device read/write:

io.UnsupportedOperation: File or stream is not seekable.

@rosjat
Copy link
Collaborator

rosjat commented Oct 31, 2023

@phako I get the reason to set the buffering there but I dont think its a good way to do it as a default this way. In genral I would leave the buffering on and make it an option to to switch it off.

so in my opinion, to not break it for older version, we could add a key word arg in init with a default that leaves buffering in place. So someone who wants no buffering can pass it in and it gets set in the open () method.

@sahlberg and @Flameeyes might can give there 5 cent about it too

@phako
Copy link
Contributor Author

phako commented Oct 31, 2023

What I understood from https://bugs.python.org/issue20074 is writing cannot work without specifying buffering=0 on python3 but maybe I got that wrong?

@rosjat
Copy link
Collaborator

rosjat commented Oct 31, 2023

this bug seems to apply to linux yes, and it seems not to be fixed but as I said before just because its something that doesnt work as expected we shouldnt make changes that restrict us from control it. So I would go for the suggested approach and in the end it wont break our code and still give ppl the ability to change it later on if this issue by python gets resolved in the future.

@phako
Copy link
Contributor Author

phako commented Feb 9, 2024

Apologies, somewhat got distracted, will provide updated patch later today

@rosjat
Copy link
Collaborator

rosjat commented Feb 12, 2024

@phako thanks for the contribution, could update the doc strings too please so it refelects your changes made to the args.

cheers

Markus

@phako
Copy link
Contributor Author

phako commented Feb 15, 2024

The code is wrong - while looking at the documentation the implementation now sets the buffer to 1 byte which is probably also not what you want. Will come back with a better solution

Enable work-around for issue when opening the device read/write on
Linux:

io.UnsupportedOperation: File or stream is not seekable.
@rosjat rosjat merged commit 0463f91 into python-scsi:master Sep 18, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants