-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Enhance hard code for export dir in deploy component #823
Enhance hard code for export dir in deploy component #823
Conversation
/assign @gaoning777 Could you please review the ticket? Thanks. |
Another place that needs modification is the notebook sample: https://github.com/kubeflow/pipelines/blob/master/samples/notebooks/KubeFlow%20Pipeline%20Using%20TFX%20OSS%20Components.ipynb. |
9992bd0
to
862828d
Compare
Addressed you comments. Great thanks for point out the missing. @gaoning777 |
/test kubeflow-pipeline-sample-test |
@gaoning777 Any more comments? Thanks for reviewing. |
I saw the sample test failure. Do you mind taking a look at this? |
Never mind. Since the new deployer component has not been built, thus sample failures with --model-export-path not found. This PR will lead to samples failures. Could we separate this PR into two:
|
862828d
to
e920f0f
Compare
@gaoning777 Thanks for your comments! |
/lgtm |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gaoning777 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 |
While using deployer component to create a TF-Serving service, hit a issue that could not find base path for servable. Checked the details, that's caused by the export directory is hard code to
model_path/export/export
as below in deploy.sh.To let the deployer component to be reusable, we should enhance that because the model export path can be specified during tensorflow/estimator training from the offical API.
For the TFX taxi sample, the export path is specified
model_path/export/export
in training step, so updatetaxi-cab-classification-pipeline.py
as well. Thanks.This change is