Add SageMaker components and example pipeline #1276
Conversation
|
Hi @Jeffwan. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
|
Hi @Jeffwan. Thanks for your PR. I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
components/aws/sagemaker/batch_transform/src/batch_transform.py
Outdated
Show resolved
Hide resolved
|
Thanks for this nice PR. It looks pretty good. My only request for it would be to add an output to the |
|
Thanks for all the feedbacks. I will clean them up |
| "ResourceConfig": { | ||
| "InstanceCount": 2, | ||
| "InstanceType": "ml.c4.8xlarge", | ||
| "VolumeSizeInGB": 50 |
There was a problem hiding this comment.
Are these hard-coded params for a pipeline user?
There was a problem hiding this comment.
Let me expose instance count and type at least. For rest of the parameters, I will wait to get some feedbacks and then make the change.
There was a problem hiding this comment.
Thanks - you can assign default values
There was a problem hiding this comment.
Got you. I make it configurable now and put default value in the example.
There was a problem hiding this comment.
Got you. I make it configurable now and put default value in the example.
BTW, you can also have default values in component.yaml:
{name: Some input, default: '60'}
There was a problem hiding this comment.
Great. I will use this to hide some details to make examples cleaner later.
Jeffwan
force-pushed
the
add_sagemaker_components
branch
from
52007ff
to
bf44cd5
Compare
Jeffwan
force-pushed
the
add_sagemaker_components
branch
from
d94e16b
to
26f2719
Compare
Update components descriptions Co-Authored-By: Jeffwan <seedjeffwan@gmail.com>
|
Thanks for a great PR |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Ark-kun 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 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Ark-kun 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 |
* Add SageMaker components and example pipeline * Address review feedbacks * Expose more training job configs * Update components/aws/sagemaker/batch_transform/component.yaml Update components descriptions Co-Authored-By: Jeffwan <seedjeffwan@gmail.com>
…w#1276) * initial readme * readme blurb * readme blurb * anchors * renaming * renaming * install prometheus operator * install prometheus operator * install prometheus operator * kfserving prefixes * prom services * readme * readme * readme * prom operator samples * v1 cluster role and binding * images * readme * Access prometheus metrics * Access prometheus metrics * minimal prometheus setup * fixed prom queries * fixed prom queries * fixed typos * restored kfserving jupyter notebook * instructions for kustomizing install namespace
Some of our users want to leverage cloud solutions so we plan to add part of aws sagemaker components and a pipeline example.
This change is