-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Adds SageMaker Ground Truth Lambda events #1447
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
Conversation
Adds events for Lambda functions for: 1. PreHumanTask 2. AnnotationConsolidation
jfuss
left a comment
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.
Two small questions/comments. Overall this looks good.
| "version": "2018-10-16", | ||
| "labelingJobArn": "arn:{{{partition}}}:sagemaker:{{{region}}}:{{{account_id}}}:labeling-job/{{{labeling_job_name}}}", | ||
| "dataObject" : { | ||
| "source-ref": "s3://{{{bucket}}}/{{{data_object_key}}}" |
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 split out bucket and data_object_key in this way? Might be easier for customers to provide the S3 uri directly.
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.
See below
| "labelAttributeName": "{{{label_attribute_name}}}", | ||
| "roleArn" : "aws:{{{partition}}}:iam::{{account_id}}:role/{{{execution_role}}}", | ||
| "payload": { | ||
| "s3Uri": "s3://{{{bucket}}}/{{{labeling_job_name}}}/annotations/worker_response/{{{iteration_object_timestamp}}}.json" |
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.
Similar here: It could make it easier for customers to just provide the bucket/labeling_job_name as one parameter instead of 2. Thoughts?
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 idea. I'll go back through and make the params match what's in CreateLabelingJob, since in some cases, the creator isn't specifying a s3 bucket, but rather a full s3 url.
Response to comments in aws#1447
|
I fixed the names in the generated event to match what what someone using the API would be passing. |
jfuss
left a comment
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.
Thanks for updating this. LGTM
Description of changes:
Adds events for Lambda functions for SageMaker Ground Truth for these Lambda functions in the CreateLabelingJob operation:
Checklist:
make prpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.