Add Pipeline Truncation & Depth Query String Support #8
Conversation
Brings over backend changes made by Kahn Academy’s appengine-mapreduce (https://github.com/Khan/appengine-mapreduce). Notably: * `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.
Adds support for `pipeline_values` de-truncation from the `status_ui` handler. Enables `depth` query string argument to enable non-root pipeline inspection.
Enables a pipeline class to provide its own value for `_DEFAULT_MAX_ATTEMPTS`.
Provide `root_pipeline_key` property to fix UI and prevent the strain required by using `_PipelineRecord.root_pipeline.get_value_for_datastore(pipeline_record)` .
Add Pipeline Truncation & Depth Query String Support
| 'Pipeline ID "%s" points to child ID "%s" which does not exist.' | ||
| % (pipeline_record.key().name(), child_pipeline_key.name())) | ||
| # If we limited the depth, we expect missing children, so don't warn. | ||
| if depth is None: |
There was a problem hiding this comment.
shouldn't we continue to raise the PipelineStatusError if depth is None?
There was a problem hiding this comment.
The children may be missing due to eventual consistency. Also, I think this is a good case for best-effort display of the data given.
See Khan@87095b9
|
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. |
|
👍 These are all great suggestions. I'll try to get these fixed up by end of week. :) |
| try: | ||
| val = unicode(value) | ||
| except UnicodeEncodeError: | ||
| val = strip_accents(value) |
There was a problem hiding this comment.
Why not just use utf8 so you know that val is always a str and never a unicode? Upstream processes (read: browsers) should be able to handle utf8 encoded strings.
if isinstance(value, basestring):
val = value.encode('utf8')
else:
val = str(value)
|
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: Khan@87095b9 |
|
@MattFaus Yes! I'll take some time to accomplish that this weekend. Thanks 👍 |
|
I'm having nightmares about this still hanging about - I'll wrap this up. 👯 |
+ Added + cleaned up docstrings + Restored missed `depthUrlArg()` refactor of `DEPTH` argument + Removed extraneous `strip_accents` method in favor of @MattFaus's unicode `.encode` suggestion + Added debug logging for non-int depth parameter
|
Ok! Changes made, ready for re-review 👍 |
|
Resolved merge conflicts. |
|
I will let @MattFaus comment on the edits (Loudr@cedb557) before I merge this in. |
surpresses bad state for abort, aborted log
|
Any news? this is quite dated :) |
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_treesupports adepthparameter, to enable non-rootpipeline lookup. Executed with new methods
db_get_in_batchesandget_pipelines_within_depth._LowMemoryPipelineRecordand_LowMemorySlotRecordadd truncationsupport for pipeline parameters.
get_pipeline_valuesprovides a means to load full values from apipeline.
_PipelineValuesHandleris a status handler that retrieves fullpipeline values for the UI.
_register_json_primitiveand the_TYPE_TO_ENCODERmap use thename of a type, rather than it’s class, for the case of unexpected
classpaths.
On the UI Side:
pipeline_valuesde-truncation from thestatus_uihandler.
depthquery string argument to enable non-root pipelineinspection.
This pull requests resolves #1 [for python] ! 👯 💃 👯