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.

Sign up for GitHub

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

Jump to bottom

TS-4523 - Add pause functionality to the TransformationPlugin interface. #712

Merged
merged 1 commit into from
Jul 20, 2017
Merged

TS-4523 - Add pause functionality to the TransformationPlugin interface. #712

merged 1 commit into from
Jul 20, 2017

Conversation

davidbz
Copy link
Contributor

@davidbz davidbz commented Jun 14, 2016

Adding the pause functionality to the TransformationPlugin.

Api changes:
Added the pause() method to the TransformationPlugin, the return value is a TSCont which you can schedule whenever you wish to resume the transformation.

@zwoop zwoop added this to the 7.0.0 milestone Jun 14, 2016
@zwoop
Copy link
Contributor

zwoop commented Jun 14, 2016

[approve ci]

@atsci
Copy link

atsci commented Jun 14, 2016

Linux build failed! See https://ci.trafficserver.apache.org/job/Github-Linux/165/ for details.

@atsci
Copy link

atsci commented Jun 14, 2016

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/271/ for details.

@zwoop
Copy link
Contributor

zwoop commented Jun 14, 2016

Remember to run clang-format on any contribution :) It's easiest done with a simple

make clang-format

@ushachar
Copy link
Contributor

A couple of comments:

  1. You should add a description to the sample plugin
  2. Since it's an API change, let's send out the email to the dev mailing list...

@davidbz
Copy link
Contributor Author

davidbz commented Jun 14, 2016

@zwoop - Thanks! it seems to work just fine on my machine.. still trying to figure it out
@ushachar - Done! thanks..

@davidbz davidbz force-pushed the TS-4523 branch 4 times, most recently from aa195dd to cde183f Compare June 16, 2016 18:35
@ushachar
Copy link
Contributor

[approve ci]

@atsci
Copy link

atsci commented Jun 16, 2016

Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/185/ for details.

@atsci
Copy link

atsci commented Jun 16, 2016

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/293/ for details.

@zwoop
Copy link
Contributor

zwoop commented Jul 23, 2016

Some minor details here:

  1. Update the commit subject message please.
  2. It now has marge conflicts, so please rebase, and re-run clang-format as well.

@ushachar Can you help land this?

@ushachar
Copy link
Contributor

ushachar commented Aug 1, 2016

Yep. Getting back to this asap....

@ushachar ushachar changed the title TS-4523 TS-4523 - Add pause functionality to the TransformationPlugin interface. Aug 16, 2016
@ushachar
Copy link
Contributor

The reason for wanting this feature is to support prolonged content related actions -- in @davidbz case out-of-process content malware analysis.

When acting as a forward proxy and handling a request for a very large file, he needs to support an initial analysis on the first N MBs of the file in order to decide if he must analyze the full file or can release it to the client.
This analysis can take a significant amount of time and continuing to buffer the contents can lead to resource exhaustion.
(When combined with turning on flow control to make sure that we throttle the proxy<->upstream tcp connection)

@bgaff - I know you had some concerns regarding this - does this use case seem reasonable to you?

@zwoop zwoop modified the milestones: 7.1.0, 7.0.0 Sep 15, 2016
@ushachar
Copy link
Contributor

ushachar commented Oct 4, 2016

[approve ci]

@atsci
Copy link

atsci commented Oct 4, 2016

FreeBSD build successful! See https://ci.trafficserver.apache.org/job/Github-FreeBSD/929/ for details.

@atsci
Copy link

atsci commented Oct 4, 2016

Linux build successful! See https://ci.trafficserver.apache.org/job/Github-Linux/823/ for details.

ushachar
ushachar suggested changes Oct 7, 2016
View reviewed changes
Copy link
Contributor

@ushachar ushachar left a comment

Choose a reason for hiding this comment

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

Can you squash the Makefile removal commit?

@davidbz
Copy link
Contributor Author

davidbz commented Oct 7, 2016

@ushachar Done. Thanks

@ushachar
Copy link
Contributor

ushachar commented Oct 7, 2016

@jpeach / @SolidWallOfCode - Since there were a couple of concerns about this API in the beginning could one of you ack this as well?

@bryancall bryancall assigned SolidWallOfCode and unassigned bgaff and ushachar Dec 16, 2016
@zwoop zwoop modified the milestones: 7.1.0, 7.2.0 Jan 8, 2017
@zwoop zwoop modified the milestones: 7.2.0, 8.0.0 Apr 25, 2017
@bryancall
Copy link
Contributor

@davidbz
Can you please rebase so we can merge it? Please make sure the commit is squashed.

@bryancall
Copy link
Contributor

[approve ci]

@davidbz
Copy link
Contributor Author

davidbz commented Jun 8, 2017

@bryancall Sure! Thanks

@davidbz
Copy link
Contributor Author

davidbz commented Jun 8, 2017

@bryancall Done.

@ushachar
Copy link
Contributor

ushachar commented Jun 8, 2017

[approve ci]

@davidbz davidbz force-pushed the TS-4523 branch 2 times, most recently from 082c2ff to a46d4c1 Compare June 10, 2017 08:46
@ushachar
Copy link
Contributor

[approve ci]

@davidbz
Copy link
Contributor Author

davidbz commented Jul 4, 2017

@bryancall Hi, commit is squashed and ready for merge :)

@bryancall bryancall merged commit fb346e1 into apache:master Jul 20, 2017
JosiahWI pushed a commit to JosiahWI/trafficserver that referenced this pull request Jul 19, 2023
@ywkaras
YTSATS-3928: webp_transform: Use Accept header from client request. (a…
dd9e3a9 
…pache#712)

Also completes transition to using TSDbg(), not TSDebug().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bryancall bryancall bryancall approved these changes

@ushachar ushachar ushachar approved these changes

Assignees

@SolidWallOfCode SolidWallOfCode

@davidbz davidbz

Labels
CPP API Plugins
Projects
None yet
Milestone
8.0.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@davidbz @zwoop @atsci @ushachar @bryancall @bgaff @SolidWallOfCode