Skip to content
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

visibility for AWS err code in addition to message #176

Merged
merged 3 commits into from
Aug 31, 2021

Conversation

surajkota
Copy link
Member

Description of changes:
Provide aws error code(e.g. Validation exception, InvalidParameterValue) in addition to message in the condition message

Testing:
Manually introduced an aws error from sdkDelete and verified it gets updated in conditions

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ack-bot ack-bot requested review from a-hilaly and vijtrip2 August 31, 2021 17:43
@surajkota surajkota requested review from jaypipes, a-hilaly and RedbackThomson and removed request for a-hilaly and vijtrip2 August 31, 2021 17:43
Copy link
Contributor

@brycahta brycahta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; will be helpful during development 👍

@brycahta
Copy link
Contributor

Description of changes:
Provide aws error code(e.g. Validation exception, InvalidParameterValue) in addition to message in the condition message

Testing:
Manually introduced an aws error from sdkDelete and verified it gets updated in conditions

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

super nit: Testing section relates to your other PR not this one

templates/pkg/resource/sdk.go.tpl Outdated Show resolved Hide resolved
@surajkota
Copy link
Member Author

super nit: Testing section relates to your other PR not this one

yes, I am using same setup for testing

@surajkota
Copy link
Member Author

@jaypipes @a-hilaly updated

@jaypipes
Copy link
Collaborator

/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label Aug 31, 2021
@ack-bot
Copy link
Collaborator

ack-bot commented Aug 31, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brycahta, jaypipes, surajkota

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-bot ack-bot merged commit eb7dc33 into aws-controllers-k8s:main Aug 31, 2021
ack-bot pushed a commit to aws-controllers-k8s/sagemaker-controller that referenced this pull request Sep 1, 2021
Description of changes:
- bring in changes from aws-controllers-k8s/code-generator#175, aws-controllers-k8s/code-generator#176
- corresponding unit and integ test changes
- fix some erroneous unit tests like model package group and endpoint config
- fix invalid name in unit test
- fix assertion in notebook instance test since PR build was failing and add it to missing canary cleanup

Testing:
unit test
tested manually, rely on PR build, later we can add more unit tests for delete failed scenario


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants