-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix: update sagemaker.serverless
integration test
#2533
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
Merged
ahsan-z-khan
merged 24 commits into
aws:master
from
bveeramani:fix-serverless-integ-test
Aug 4, 2021
Merged
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
fab0196
Rename `delete_endpoint` as `delete_predictor`
bveeramani bc5a586
Update test_serverless.py
bveeramani 5f1131c
Update test_serverless.py
bveeramani 837b45e
Merge branch 'master' into fix-serverless-integ-test
bveeramani 2689fd6
Appease lint
bveeramani 98a8556
Merge branch 'fix-serverless-integ-test' of github.com:bveeramani/sag…
bveeramani 62b6bf7
Merge branch 'master' into fix-serverless-integ-test
shreyapandit ba91b52
Update serverless test
bveeramani 34cb7cc
Merge branch 'master' into fix-serverless-integ-test
bveeramani 9ef49e3
Appease lint
bveeramani bcfe280
Merge branch 'fix-serverless-integ-test' of github.com:bveeramani/sag…
bveeramani 8b017e8
Update test_serverless.py
bveeramani 9dff637
Merge branch 'master' into fix-serverless-integ-test
bveeramani 9075a44
Merge branch 'master' into fix-serverless-integ-test
shreyapandit b522f53
Merge branch 'master' into fix-serverless-integ-test
bveeramani 8e93bde
Appease lint
bveeramani 31daaea
Merge branch 'fix-serverless-integ-test' of github.com:bveeramani/sag…
bveeramani f9f07bb
Fix test
bveeramani e35ad7e
Update test_serverless.py
bveeramani e8c59e5
Update test_serverless.py
bveeramani 42fdd2b
Update test_serverless.py
bveeramani b2156fd
Update test_serverless.py
bveeramani c161878
Fix tests
bveeramani 318b1d7
Merge branch 'master' into fix-serverless-integ-test
bveeramani File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Do we need to create these two fixtures?
Can't we just use
boto_session
to accessaccount
andregion
?Any benefit for creating this one?
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.
For
account
, it's to reduce code duplication. While it's true we could useboto_session.client("sts").get_caller_identity()["Account"]
every time we want to access the account ID, IMO setting up a fixture for the account is a cleaner implementation (theaccount
fixture is used more than once).With respect to the
region
fixture, there's less of a case for that. I subjectively feel it simplifies the implementation, but I also see the argument for why it may be unnecessary. If you feel that we should get rid of it, I'd be okay with doing so.