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

elements for batch and tf protocol #3280

Merged
merged 2 commits into from
Jun 11, 2021

Conversation

ryandawsonuk
Copy link
Contributor

fixes #3279

@@ -135,11 +135,19 @@ def process_and_update_elastic_doc(

no_items_in_batch = len(new_content_part["instance"])
index = 0
for item in new_content_part["instance"]:
elements = None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was just iterating through the instances in case of batch... now we need to iterate through instances and find corresponding 'elements' for each but only if there are elements (might not be if there are no names and no schema)

namespace = log_helper.get_header(log_helper.NAMESPACE_HEADER_NAME, headers)
inferenceservice_name = log_helper.get_header(log_helper.INFERENCESERVICE_HEADER_NAME, headers)
endpoint_name = log_helper.get_header(log_helper.ENDPOINT_HEADER_NAME, headers)
serving_engine = log_helper.serving_engine(headers)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

moving this up so it can be used across the code for different protocols

i, requestMsg, req_datatype, req_features, req_datadef
)
reqJson["elements"] = e
copy = reqJson
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this code to iterate elements is replaced by the new logic at line 138


item_body = doc_body.copy()

item_body[message_type]["instance"] = item

if type(elements) == type([]) and len(elements) >= num:
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this condition for length >= num?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I think it should be >. It is checking that elements array contains an entry at this index. So if at index 2 then elements needs to be of length of at least three. I will try changing it.

Copy link
Contributor

@SachinVarghese SachinVarghese left a comment

Choose a reason for hiding this comment

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

/lgtm

@seldondev seldondev added size/L and removed size/M labels Jun 11, 2021
@seldondev
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SachinVarghese

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@seldondev seldondev merged commit 1e504dc into SeldonIO:master Jun 11, 2021
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.

req logger - create elements section for tensorflow protocol
3 participants