-
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 docstrings to resource actions and sub-resources #239
Conversation
333f797
to
ef5f96d
Compare
The most recent updates add docstrings to identifiers, attributes, and references. Note that tests still won't pass until all of the botocore docstring PRs are merged. |
ef5f96d
to
b6f6e8e
Compare
Cool. Since all of the botocore docstrings have been merged, this should be good to get reviewed. No longer relying on any outstanding PRs. |
self._session = session | ||
# I know that this is an internal attribute, but the botocore session | ||
# is needed to load the paginator and waiter models. | ||
self._botocore_session = session._session |
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've noticed there's been a few scenarios where we need something from botocore's session. Should we start exposing some of the botocore session methods onto a boto3 session?
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.
We could look into this. I checked on where I use the botocore session. I use it for grabbing paginator models, waiter models, and the loader. Those are pretty low level as well so I am not sure if we want to expose those quite yet. I am not sure adding these to the boto3 session is in the scope of the this pr. Thoughts?
Looks good, just had a few comments. |
@jamesls I updated based on feedback. I did have a comment on exposing more methods on the boto3 session. I feel we should talk more about if the methods that we would need to expose to prevent accessing the |
|
Add docstrings to resource actions and sub-resources
This adds lazy loaded docstrings to resource actions and sub-resources. Note that I still have yet to add docstrings identifiers, attributes, collections, and resource waiters. I just figured that splitting the work up a bit would be easier to review. There are a lot of moving parts so I figured to break it up to make it easier to follow and track feedback updates. Here are some examples of what you can do now:
Note that the tests won't pass till this PR gets merged: boto/botocore#642
cc @jamesls @mtdowling @rayluo