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

DRAFT: YAN-913 Async Drop Streaming #134

Closed
wants to merge 1,743 commits into from
Closed

DRAFT: YAN-913 Async Drop Streaming #134

wants to merge 1,743 commits into from

Conversation

calgray
Copy link
Collaborator

@calgray calgray commented Mar 29, 2022

This change adds an alternative approach to stream buffering that utilizes dataDrops for controlling the stream backbuffer rather than passing directly to the next appdrop via callback. The motive for buffering is to allow subprocesses to asynchronously process streams rather than immediately invoke a chain of callbacks, all of which would be executed by the first streaming drop process.

james-strauss-uwa and others added 30 commits February 2, 2022 15:46
@coveralls
Copy link

coveralls commented Mar 29, 2022

Coverage Status

Coverage remained the same at 66.698% when pulling a7c875b on YAN-913-streaming into d5947a5 on master.

def _open(self, **kwargs):
if self._mode == OpenMode.OPEN_WRITE:
return self._buf
elif self._mode == OpenMode.OPEN_READ:
# TODO: potentially wasteful copy
return io.BytesIO(self._buf.getbuffer())
br = MemoryIO.BytesIOReader(self._buf)
Copy link
Collaborator Author

@calgray calgray Mar 29, 2022

Choose a reason for hiding this comment

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

One of the issues I've noticed here this is that numerous read functions call close() on the io object after reading. When updating such that multiple readers all point to the same _buf object, the reader closing additionally closes the bytesio object, causing all other drop reads to error.

Abstracting the reader close to just the BytesIOReader should allow DataDrop to continue correctly handling the closing of the underlying BytesIO object.

@@ -82,7 +82,7 @@ def initialize(self, **kwargs):
self.config_data = self.serialize_parameters(
self.filter_parameters(self.parameters, self.mode), self.mode).encode('utf-8')

def getIO(self):
def _getIO(self):
Copy link
Collaborator Author

@calgray calgray Mar 29, 2022

Choose a reason for hiding this comment

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

A rather large refactor, but I haven't found an existing case where the DataDROP interface isn't sufficient for an app and needs to use DataIO directly.

To add drop streaming I've simply added dataDrop.writeStream() and dataDrop.readStream()

@calgray calgray changed the title YAN-913 drop asnyc streaming YAN-913 Async Drop Streaming Mar 29, 2022
@calgray calgray changed the title YAN-913 Async Drop Streaming DRAFT: YAN-913 Async Drop Streaming Mar 29, 2022
@awicenec awicenec force-pushed the master branch 2 times, most recently from eb80e22 to 06cd2c3 Compare May 19, 2022 12:50
@calgray calgray self-assigned this May 27, 2022
@calgray
Copy link
Collaborator Author

calgray commented May 27, 2022

Rebased in #165

@calgray calgray closed this May 27, 2022
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.

None yet

6 participants