Convert to Elasticsearch streaming_bulk helper #2492
Conversation
There was a problem hiding this comment.
You should update the PR description (e.g., remove the references to it being a draft), and the branch needs a rebase...but, this is great stuff!
Of course, I have the usual pile of comments. The principal theme running through them is that this code is evidently evolutionary: there are vestiges of the original code in the new code, and there are vestiges of the subclass in the superclass; I've pointed out the ones that I found. Most of my concerns are centered around documentation and comments, particularly around the json_data parameter, which turns out to be a bit of a swiss-army knife. (It's really more like a "context" parameter.)
So, really, it just needs a little polish and then it will be good to go!
lib/pbench/test/unit/server/query_apis/test_datasets_publish.py
Outdated
Show resolved
Hide resolved
lib/pbench/test/unit/server/query_apis/test_datasets_publish.py
Outdated
Show resolved
Hide resolved
lib/pbench/test/unit/server/query_apis/test_datasets_publish.py
Outdated
Show resolved
Hide resolved
Simplify the pre-update info log to speed things up and avoid an unnecessary pass through the MAP; the document count will appear in the post-update message anyway. Add a clarification that the bulk update operation only changes the one specified field.
Elasticsearch bulk "action" rather than assuming "update" in the base class.
|
@webbnh still has an unresolved conversation. |
This creates a new
ElasticBulkBaseclass providing support for Elasticsearch bulk operations including publish and delete, and which currently shares the query_apis__init__.pywith theElasticBaseclass.Resolves #2490