SageMaker Groundtruth module added and tested #199
Conversation
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
… dev_sagemaker_ground_truth
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
Where are all the assets from? Are they available under the appropriate license of this repository?
...b/assets/images/2021-01-28-09-50-34/bad/image-2020-04-22-10-51-14-06-cropped-left-bottom.jpg
Outdated
Show resolved
Hide resolved
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
There was a problem hiding this comment.
Looks like there's still some test images left in the PR.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
modules/__init__.py
Outdated
There was a problem hiding this comment.
I think we don't want to add these extra __init__.py files as this doesn't represent a Python package. (Same goes for modules/sagemaker/__init__.py.)
|
|
||
|
|
||
| ## Deployment | ||
| The deployment of the required assets (Lambda functions, IAM roles, etc.) for the Step Functions workflow is automated using an AWS CDK app. The pipeline is triggered on a schedule as well as on Git commits, using two pipelines in AWS CodePipeline. The architecture of the CI/CD infrastructure deployed by the CDK app is as follows: |
There was a problem hiding this comment.
Is this missing a further description of the Ci/CD infrastructure?
| #### Required | ||
| - Repository Details: | ||
|
|
||
| - assets bucket: The name of S3 bucket with assets for the lambda functions and images for labelling. |
There was a problem hiding this comment.
This doesn't seem to be a configuration option in settings.py?
| ## Inputs/Outputs | ||
|
|
||
| ### Input Parameters | ||
| #### Required |
There was a problem hiding this comment.
It would help to denote the configuration names with tick marks to make them code!
| - repoName: The name of the repository where the source code is stored. | ||
| - branchName: The branch name to use from the repository. |
There was a problem hiding this comment.
I think the modules usually use kebab-case for parameters in the documentation (branch-name) and then snake-case in the code (branch_name).
| pre_build: | ||
| commands: | ||
| - cdk destroy --force --app "python app.py" | ||
| - echo "Prebuild stage" | ||
| build: | ||
| commands: | ||
| - echo "DESTROY!" | ||
| post_build: | ||
| commands: | ||
| - echo "Destroy successful" |
There was a problem hiding this comment.
| pre_build: | |
| commands: | |
| - cdk destroy --force --app "python app.py" | |
| - echo "Prebuild stage" | |
| build: | |
| commands: | |
| - echo "DESTROY!" | |
| post_build: | |
| commands: | |
| - echo "Destroy successful" | |
| build: | |
| commands: | |
| - cdk destroy --force --app "python app.py" |
There was a problem hiding this comment.
Should this be reduced to only the 10 or so images that we have in the PR now?
| model_package_group_description: str = Field( | ||
| "Contains models for quality inspection of metal tags" | ||
| ) | ||
| repoType: str = Field("CODECOMMIT") |
There was a problem hiding this comment.
We'll get type checking for free if we use an enum:
from enum import Enum
class RepoEnum(str, Enum):
code_commit = 'CODECOMMIT'
github = 'GITHUB'| repoType: str = Field("CODECOMMIT") | |
| repoType: RepoEnum = RepoEnum.code_commit |
There was a problem hiding this comment.
It might make more sense to provide a Python file that defines AppConfig to use instead of going from YAML -> loading it in Python -> renaming the fields to be something else.
There was a problem hiding this comment.
I think the images are included twice -- in both lib/assets and tests/lib/assets. Why do we include them in both places? Can we only include them once?
… dev_sagemaker_ground_truth
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Closing as refactored in #252 |
Describe your changes
Issue ticket number and link
Checklist before requesting a review
CHANGELOG.MDwith a description of my changesscripts/validate.sh)seedfarmer applyBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.