-
Notifications
You must be signed in to change notification settings - Fork 258
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
SageMaker endpoint: enhance integ test coverage for failure conditions #730
Conversation
These helper methods should make things a lot cleaner, and the fixtures much more clearly show the test setup as compared to our previous system. Thank you! |
3c21e1f
to
e0345b4
Compare
d3d9ed8
to
53004f0
Compare
53004f0
to
c67b80c
Compare
|
||
return None | ||
|
||
def assert_condition_state_message(reference: CustomResourceReference, |
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.
Not seeing where assert_condition_state_message is used
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.
test/e2e/sagemaker/tests/test_endpoint.py line#324
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.
I presume a lot of the style-only changes are due to running black
on the files, so this is fine. In the future, consider submitting separate PRs with style-only changes since that helps reviewers know what is a functional change and what isn't.
Description of changes:
retainAllVariantProperties
used in udpateEndpoint code pathk8s.load_and_create_resource
and related methodswait_for_condition
pre-maturely exited if the condition is nil. Depending on the case the condition maybe set later_get_terminal_condition
andis_resource_in_terminal_condition
implementations was not correct, replaced with generic methodsTesting
Locally using pytest
Related PR: aws-controllers-k8s/sagemaker-controller#7
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.