appengine-pipelines icon indicating copy to clipboard operation
appengine-pipelines copied to clipboard

Add Pipeline Truncation & Depth Query String Support

Open soundofjw opened this issue 11 years ago • 9 comments

Brings over UI updates and truncation support originally implemented by Kahn Academy’s appengine-mapreduce repo (https://github.com/Khan/appengine-mapreduce).

For the python backend:

  • get_status_tree supports a depth parameter, to enable non-root pipeline lookup. Executed with new methods db_get_in_batches and get_pipelines_within_depth.
  • _LowMemoryPipelineRecord and _LowMemorySlotRecord add truncation support for pipeline parameters.
  • get_pipeline_values provides a means to load full values from a pipeline.
  • _PipelineValuesHandler is a status handler that retrieves full pipeline values for the UI.
  • _register_json_primitive and the _TYPE_TO_ENCODER map use the name of a type, rather than it’s class, for the case of unexpected classpaths.

On the UI Side:

  • Adds support for pipeline_values de-truncation from the status_ui handler.
  • Enables depth query string argument to enable non-root pipeline inspection.

This pull requests resolves #1 [for python] ! :dancers: :dancer: :dancers:

soundofjw avatar Nov 16 '14 19:11 soundofjw

Hey, I've left some comments on the request. No huge changes, just nitpicks and requests for documentation/clarification. Overall this looks very backwards-compatible while adding valuable functionality to the UI for large pipeline jobs. Thanks for porting the code over here.

AngryBrock avatar Nov 17 '14 19:11 AngryBrock

:+1: These are all great suggestions. I'll try to get these fixed up by end of week. :)

soundofjw avatar Nov 19 '14 23:11 soundofjw

Thanks so much for doing this porting! I'm finally getting around to reworking our repo such that it is a proper fork of this one. Will you address the comments so we can get this pulled in and others can benefit?

For posterity the changes in this pull request correspond to these commits in our repo:

https://github.com/Khan/appengine-pipelines/commit/87095b904f42d3788280bd1b0654887e8f8c7267 https://github.com/Khan/appengine-pipelines/commit/b43f8ed3a74a4aac5a479e15df08aa5527d429f3 https://github.com/Khan/appengine-pipelines/commit/6db6c6e960c414d1273c4728c390189b1b2fa41f https://github.com/Khan/appengine-pipelines/commit/b31ad76aaae036536e9b36e33948e8b459ac9e9f https://github.com/Khan/appengine-pipelines/commit/6f9b30e2a2e44b3c31561207b72cecd0203c484c https://github.com/Khan/appengine-pipelines/commit/9e0b62fd4d8e786595c02f2f2c80a714da6e7f8e

MattFaus avatar Jun 11 '15 21:06 MattFaus

@MattFaus Yes! I'll take some time to accomplish that this weekend. Thanks :+1:

soundofjw avatar Jun 12 '15 16:06 soundofjw

I'm having nightmares about this still hanging about - I'll wrap this up. This functionality saves us a lot of time. :]

:dancers:

soundofjw avatar Jul 28 '15 21:07 soundofjw

Ok! Changes made, ready for re-review :+1:

soundofjw avatar Jul 28 '15 21:07 soundofjw

Resolved merge conflicts.

soundofjw avatar Jul 31 '15 20:07 soundofjw

I will let @MattFaus comment on the edits (https://github.com/Loudr/appengine-pipelines/commit/cedb5570f4c136ef1a542b30a85139701b524982) before I merge this in.

AngryBrock avatar Aug 05 '15 15:08 AngryBrock

Any news? this is quite dated :)

soundofjw avatar Jun 15 '16 18:06 soundofjw