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

Add streaming API #9

Merged
merged 18 commits into from
Jul 12, 2015
Merged

Add streaming API #9

merged 18 commits into from
Jul 12, 2015

Conversation

clue
Copy link
Owner

@clue clue commented Jun 15, 2015

Refs #4

@adamlc
Copy link

adamlc commented Jun 17, 2015

Looks good!

@clue
Copy link
Owner Author

clue commented Jun 17, 2015

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
Copy link
Owner Author

clue commented Jun 17, 2015

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
Copy link
Owner Author

clue commented Jun 18, 2015

Remaining TODOs:

  • Streaming API for containerPush endpoint
  • Documentation

@clue
Copy link
Owner Author

clue commented Jun 19, 2015

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
Copy link
Owner Author

clue commented Jun 21, 2015

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

Any feedback is very much appreciated! 👍

@adamlc
Copy link

adamlc commented Jun 22, 2015

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

@clue
Copy link
Owner Author

clue commented Jun 30, 2015

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
Copy link
Owner Author

clue commented Jul 10, 2015

I think this is finally completed 👏

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

@clue
Copy link
Owner Author

clue commented Jul 12, 2015

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants