-
Notifications
You must be signed in to change notification settings - Fork 835
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
Issue/1638 model metadata #1671
Issue/1638 model metadata #1671
Conversation
Check out this pull request on You'll be able to see Jupyter notebook diff and discuss changes. Powered by ReviewNB. |
Tue Apr 7 11:31:06 UTC 2020 impatient try |
Tue Apr 7 11:31:17 UTC 2020 impatient try |
Tue Apr 7 13:33:46 UTC 2020 impatient try |
Tue Apr 7 13:33:51 UTC 2020 impatient try |
Wed Apr 8 14:40:38 UTC 2020 impatient try |
Wed Apr 8 14:40:40 UTC 2020 impatient try |
Wed Apr 8 22:33:20 UTC 2020 impatient try |
Wed Apr 8 22:33:31 UTC 2020 impatient try |
Wed Apr 8 22:40:32 UTC 2020 impatient try |
Wed Apr 8 22:41:55 UTC 2020 impatient try |
Wed Apr 8 22:43:45 UTC 2020 impatient try |
Wed Apr 8 22:45:06 UTC 2020 impatient try |
Wed Apr 8 22:45:07 UTC 2020 impatient try |
Wed Apr 8 22:46:58 UTC 2020 impatient try |
Wed Apr 8 23:07:25 UTC 2020 impatient try |
Wed Apr 8 23:07:28 UTC 2020 impatient try |
Tue Apr 14 13:37:30 UTC 2020 impatient try |
Tue Apr 14 13:37:32 UTC 2020 impatient try |
Wed Apr 15 15:09:36 UTC 2020 impatient try |
Wed Apr 15 15:09:41 UTC 2020 impatient try |
Wed Apr 15 15:15:04 UTC 2020 impatient try |
Wed Apr 15 15:15:14 UTC 2020 impatient try |
Thu Apr 16 08:46:10 UTC 2020 impatient try |
Thu Apr 16 08:46:11 UTC 2020 impatient try |
Thu Apr 16 08:48:58 UTC 2020 impatient try |
Wed Apr 22 16:23:16 UTC 2020 impatient try |
Wed Apr 22 16:58:19 UTC 2020 impatient try |
Wed Apr 22 16:58:24 UTC 2020 impatient try |
Wed Apr 22 17:08:26 UTC 2020 impatient try |
Wed Apr 22 17:08:30 UTC 2020 impatient try |
/test integration |
Wed Apr 22 17:12:29 UTC 2020 impatient try |
Wed Apr 22 17:12:31 UTC 2020 impatient try |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, only a couple of minor comments
{"name": "my-model-name", | ||
"versions": ["my-model-version-02"], | ||
"platform": "seldon-custom", | ||
"inputs": [{"name": "input", "datatype": "BYTES", "shape": [1]}], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is it bytes?
""" | ||
|
||
def __init__(self, name: str, datatype: str, shape: Union[List[int]]): | ||
self.name = str(name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here it would still be name "None"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only if someone call if with None
, here there is no default value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point
/retest |
Wed Apr 22 20:14:41 UTC 2020 impatient try |
Looks good, if we confirm that notebook tests failing are flaky should be good to land! |
Wed Apr 22 22:05:28 UTC 2020 impatient try |
Did you manage to see in jx logs which notebook test failed? I can only see
:( |
/test notebooks |
Thu Apr 23 08:31:32 UTC 2020 impatient try |
Thu Apr 23 09:46:52 UTC 2020 impatient try |
Thu Apr 23 09:47:01 UTC 2020 impatient try |
All notebook tests passed for me locally. Only customMetrics one didn't like that I have zsh as my underlying shell (kubectl run had problems) so I changed that part into a short shell script called explicitly with bash. |
@RafalSkolasinski: The following test failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
In CI it failed on |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cliveseldon 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 |
closes #1638
Deprecates
metadata
method in Python wrapper and introduceinit_metadata
method.The
init_metadata
will be called once after model initialisation to validate and cache returned metadata.Output of
init_metadata
is merged with content ofMODEL_METADATA
environmental variable with values in latter taking priorities.For prepackaged model server (right now only sklearn) one can either define
MODEL_METADATA
environmental variables or addmetadata.yaml
file to the s3 bucket. Please see examples.