You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@liminal.apache.org by GitBox <gi...@apache.org> on 2020/10/20 13:13:04 UTC
[GitHub] [incubator-liminal] aviemzur opened a new pull request #3: adding demo app
aviemzur opened a new pull request #3:
URL: https://github.com/apache/incubator-liminal/pull/3
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-liminal] aviemzur commented on a change in pull request #3: adding demo app
Posted by GitBox <gi...@apache.org>.
aviemzur commented on a change in pull request #3:
URL: https://github.com/apache/incubator-liminal/pull/3#discussion_r511566047
##########
File path: liminal/build/service/python_server/liminal_python_server.py
##########
@@ -27,7 +27,10 @@ def start_server(yml_path):
__start_server(yaml.safe_load(stream))
+# noinspection PyUnresolvedReferences
def __start_server(config):
+ globals()['request'] = __import__('flask').request
Review comment:
I need the lambda below to pass request data to user code, and this was the only way I could load it and have it available when the lambda below runs.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-liminal] aviemzur commented on pull request #3: adding demo app
Posted by GitBox <gi...@apache.org>.
aviemzur commented on pull request #3:
URL: https://github.com/apache/incubator-liminal/pull/3#issuecomment-712839785
@assapin
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-liminal] assapin commented on a change in pull request #3: adding demo app
Posted by GitBox <gi...@apache.org>.
assapin commented on a change in pull request #3:
URL: https://github.com/apache/incubator-liminal/pull/3#discussion_r510233722
##########
File path: liminal/build/service/python_server/liminal_python_server.py
##########
@@ -27,7 +27,10 @@ def start_server(yml_path):
__start_server(yaml.safe_load(stream))
+# noinspection PyUnresolvedReferences
def __start_server(config):
+ globals()['request'] = __import__('flask').request
Review comment:
why are you doing the import in this way?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-liminal] aviemzur commented on a change in pull request #3: adding demo app
Posted by GitBox <gi...@apache.org>.
aviemzur commented on a change in pull request #3:
URL: https://github.com/apache/incubator-liminal/pull/3#discussion_r511566152
##########
File path: tests/runners/airflow/demo/model_store.py
##########
@@ -0,0 +1,45 @@
+import pickle
+import time
+
+import boto3
+
+BUCKET = 'ni-ml-ab-dev'
Review comment:
Wouldn't work for them, they would need an s3 bucket
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-liminal] aviemzur closed pull request #3: adding demo app
Posted by GitBox <gi...@apache.org>.
aviemzur closed pull request #3:
URL: https://github.com/apache/incubator-liminal/pull/3
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-liminal] assapin commented on a change in pull request #3: adding demo app
Posted by GitBox <gi...@apache.org>.
assapin commented on a change in pull request #3:
URL: https://github.com/apache/incubator-liminal/pull/3#discussion_r510237929
##########
File path: tests/runners/airflow/demo/model_store.py
##########
@@ -0,0 +1,45 @@
+import pickle
+import time
+
+import boto3
+
+BUCKET = 'ni-ml-ab-dev'
Review comment:
how will this example work for users? in terms of requiring S3 buckets to exist etc.?
##########
File path: tests/runners/airflow/demo/serving.py
##########
@@ -0,0 +1,16 @@
+import json
+
+import model_store
+from model_store import ModelStore
+
+_MODEL_STORE = ModelStore(model_store.PRODUCTION)
+_PETAL_WIDTH = 'petal_width'
+
+
+def predict(input_json):
Review comment:
suggest to name the file prediction.py or inference.py
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-liminal] assapin commented on pull request #3: adding demo app
Posted by GitBox <gi...@apache.org>.
assapin commented on pull request #3:
URL: https://github.com/apache/incubator-liminal/pull/3#issuecomment-714561606
Generally I suggest to think of this app as the first use-case in our /examples folder like most projects do
Need to think about making the yaml more explainable (naming)
and adding instructions for use of S3 (inc. if needed creating buckets...)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-liminal] assapin commented on a change in pull request #3: adding demo app
Posted by GitBox <gi...@apache.org>.
assapin commented on a change in pull request #3:
URL: https://github.com/apache/incubator-liminal/pull/3#discussion_r510237371
##########
File path: tests/runners/airflow/demo/liminal.yml
##########
@@ -0,0 +1,37 @@
+---
+name: MyDataScienceApp
Review comment:
I suggest to move this away from the "tests" and into "examples"
Also go over the yaml and change names to more meaningful ones (instead of foo/baz and my server) to make it easier to follow
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org