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

Fix tags propogation with seldon client #3374

Merged
merged 6 commits into from
Jul 8, 2021

Conversation

majolo
Copy link
Contributor

@majolo majolo commented Jul 6, 2021

I discovered two small bugs in the seldon client when testing the batch processor.

  • Tags end up being wrapped twice in the response, so the result is ["meta"]["tags"]["tags"]
  • Meta isn't propagated when the gateway is seldon not istio

@majolo
Copy link
Contributor Author

majolo commented Jul 6, 2021

/test integration

1 similar comment
@RafalSkolasinski
Copy link
Contributor

/test integration

@axsaucedo
Copy link
Contributor

Nice one
/test integration

Copy link
Contributor

@RafalSkolasinski RafalSkolasinski left a comment

Choose a reason for hiding this comment

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

Nice one!

raise RuntimeError(
"Only `ndarray` and `tensor` input are currently supported for batch size greater than 1."
)
except Exception as e:
Copy link
Contributor

Choose a reason for hiding this comment

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

Not clear what exception are we trying to caught here

Copy link
Contributor

Choose a reason for hiding this comment

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

But looking at other parts of code they follow same pattern so I guess it is okay for now.

@@ -267,7 +267,7 @@ def _send_batch_predict_multi_request(
retries: int,
batch_id: str,
payload_type: str,
) -> str:
) -> [str]:
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this coming from lint?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No it was a warning from my IDE

@seldondev
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RafalSkolasinski

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 87d78e7 into SeldonIO:master Jul 8, 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.

4 participants