Skip to content

Add streaming API#9

Merged
clue merged 18 commits into
clue:masterfrom
clue-labs:streaming
Jul 12, 2015
Merged

Add streaming API#9
clue merged 18 commits into
clue:masterfrom
clue-labs:streaming

Conversation

@clue

@clue clue commented Jun 15, 2015

Copy link
Copy Markdown
Owner

Refs #4

@clue clue added help wanted Extra attention is needed new feature New feature or request labels Jun 15, 2015
@clue clue mentioned this pull request Jun 15, 2015
@adamlc

adamlc commented Jun 17, 2015

Copy link
Copy Markdown

Looks good!

@clue

clue commented Jun 17, 2015

Copy link
Copy Markdown
Owner Author

Thanks for your feedback!

In order to get this in faster, this PR will only address adding a streaming API for existing endpoints, it is not about adding additional endpoints.

This PR lacks some documentation.

The existing endpoints containerExport() and containerCopy() return streams in the TAR file format and afaict there's no easy way to consume these via React PHP. As such, I've looked into creating a library over at https://github.com/clue/php-tar-react . We won't necessarily have to include this here, but we should provide some documentation. This PR will be postponed for now.

@clue

clue commented Jun 17, 2015

Copy link
Copy Markdown
Owner Author

I've looked into creating a library over at https://github.com/clue/php-tar-react . We won't necessarily have to include this here, but we should provide some documentation. This PR will be postponed for now.

I've just released an early v0.1.0 of clue/tar-react, so I can now continue looking into this PR :)

@clue

clue commented Jun 18, 2015

Copy link
Copy Markdown
Owner Author

Remaining TODOs:

  • Streaming API for containerPush endpoint
  • Documentation

@clue

clue commented Jun 19, 2015

Copy link
Copy Markdown
Owner Author

I've just added caret notation encoding to the copy/export example, so that dumping binary files does not screw up your terminal.

Now getting back to the last missing tests..

@clue clue changed the title [WIP] Add streaming API Add streaming API Jun 21, 2015
@clue

clue commented Jun 21, 2015

Copy link
Copy Markdown
Owner Author

There you go, I've just completed the last TODOs and removed the WIP marker.

Any feedback is very much appreciated! 👍

@adamlc

adamlc commented Jun 22, 2015

Copy link
Copy Markdown

Awesome! I'll try and give it a whirl over the next couple of days!

@clue

clue commented Jun 30, 2015

Copy link
Copy Markdown
Owner Author

Awesome! I'll try and give it a whirl over the next couple of days!

Awesome, let me know if you get to this!

Two remaining issues:

@clue

clue commented Jul 10, 2015

Copy link
Copy Markdown
Owner Author

I think this is finally completed 👏

This is a non-trivial feature addition, so this can probably use another review :)

@clue

clue commented Jul 12, 2015

Copy link
Copy Markdown
Owner Author

This is a non-trivial feature addition, so this can probably use another review :)

Either way, let's get this in :shipit:

clue added a commit that referenced this pull request Jul 12, 2015
@clue clue merged commit 8efb578 into clue:master Jul 12, 2015
@clue clue deleted the streaming branch July 12, 2015 10:35
@clue clue added this to the v0.2.0 milestone Apr 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted Extra attention is needed new feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants