Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add custom load() for s3.Bucket #128

Merged
merged 4 commits into from
Jun 16, 2015
Merged

Conversation

jamesls
Copy link
Member

@jamesls jamesls commented Jun 9, 2015

Would like some feedback on whether we want this change. This is proposed from #102.

It does solve the problem that you will no longer get a confusing error when accessing bucket.creation_date, but we have to make a ListBuckets requests to get this information.

cc @kyleknap @mtdowling @trevorrowe

@jamesls
Copy link
Member Author

jamesls commented Jun 10, 2015

Test fixed. Should be ready to look at now.

@jamesls jamesls added the bug This issue is a confirmed bug. label Jun 10, 2015


def bucket_load(self, *args, **kwargs):
"""Fake s3.Bucket.load method.
Copy link
Contributor

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 and reload methods as the following:

Calls [service].Client.[method]() to update the attributes of the [name_of_resource] resource.

So maybe do it as?

Calls s3.Client.list_objects() to update the attributes of the Object resource.

Then you can move the original docstring to a comment or into the inject method.

Let me know what you think.

Copy link
Member Author

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.

@kyleknap
Copy link
Contributor

Looks good. I just had a few comments. 🚢

@jamesls
Copy link
Member Author

jamesls commented Jun 15, 2015

Feedback incorporated.

@kyleknap
Copy link
Contributor

Thanks. 🚢

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug This issue is a confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants