-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Add custom load() for s3.Bucket #128
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -279,6 +279,10 @@ def _register_default_handlers(self): | |
'creating-client-class.s3', | ||
boto3.utils.lazy_call( | ||
'boto3.s3.inject.inject_s3_transfer_methods')) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On a complete side note, we should look into adding |
||
self._session.register( | ||
'creating-resource-class.s3.Bucket', | ||
boto3.utils.lazy_call( | ||
'boto3.s3.inject.inject_bucket_load')) | ||
self._session.register( | ||
'creating-resource-class.dynamodb', | ||
boto3.utils.lazy_call( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
import shutil | ||
import hashlib | ||
import string | ||
import datetime | ||
|
||
from tests import unittest, unique_id | ||
from botocore.compat import six | ||
|
@@ -460,11 +461,41 @@ def test_transfer_methods_through_client(self): | |
assert_files_equal(filename, download_path) | ||
|
||
|
||
class TestS3TransferMethodInjection(unittest.TestCase): | ||
class TestS3MethodInjection(unittest.TestCase): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test class looks more like it belongs in the the functional test category because you are only checking if it has a particular attribute. |
||
def test_transfer_methods_injected_to_client(self): | ||
session = boto3.session.Session(region_name='us-west-2') | ||
client = session.client('s3') | ||
self.assertTrue(hasattr(client, 'upload_file'), | ||
'upload_file was not injected onto S3 client') | ||
self.assertTrue(hasattr(client, 'download_file'), | ||
'download_file was not injected onto S3 client') | ||
|
||
def test_bucket_resource_has_load_method(self): | ||
session = boto3.session.Session(region_name='us-west-2') | ||
bucket = session.resource('s3').Bucket('fakebucket') | ||
self.assertTrue(hasattr(bucket, 'load'), | ||
'load() was not injected onto S3 Bucket resource.') | ||
|
||
|
||
class TestCustomS3BucketLoad(unittest.TestCase): | ||
def setUp(self): | ||
self.region = 'us-west-2' | ||
self.session = boto3.session.Session(region_name=self.region) | ||
self.s3 = self.session.resource('s3') | ||
self.bucket_name = unique_id('boto3-test') | ||
|
||
def create_bucket_resource(self, bucket_name, region=None): | ||
if region is None: | ||
region = self.region | ||
kwargs = {'Bucket': bucket_name} | ||
if region != 'us-east-1': | ||
kwargs['CreateBucketConfiguration'] = { | ||
'LocationConstraint': region | ||
} | ||
bucket = self.s3.create_bucket(**kwargs) | ||
self.addCleanup(bucket.delete) | ||
return bucket | ||
|
||
def test_can_access_buckets_creation_date(self): | ||
bucket = self.create_bucket_resource(random_bucket_name()) | ||
self.assertIsInstance(bucket.creation_date, datetime.datetime) |
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.
If this is a custom method, I will use this docstring because it is not modeled in the resource model.
I have been auto-documenting the
load
andreload
methods as the following:So maybe do it as?
Then you can move the original docstring to a comment or into the inject method.
Let me know what you think.
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.
Sounds good, I'll update.