-
Notifications
You must be signed in to change notification settings - Fork 10
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 debug information to the S3 adapter #45
Conversation
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 left a few minor nits and suggestions but LGTM! 💪 This makes for a great and thorough addition to the docs and looks good when I build it locally.
administrators.rst
Outdated
There is more information about overriding the Storage Service defaults in the | ||
`installation README.md`_. | ||
|
||
In the logging configuration, administrators should be able to find entries for |
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.
Maybe worth linking to the logging configuration file in the source code?
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.
Nice idea, will do!
administrators.rst
Outdated
Debugging S3 | ||
"""""""""""" | ||
|
||
There are times when the S3 storage adapter needs to be debugged. For example, |
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.
Maybe "may need to be debugged" would be more accurate? Should work out of the box for at least some folks.
From the documentation, the `Boto3 developers`_ are careful to note as follows: | ||
|
||
.. warning:: | ||
|
||
Be aware that when logging anything from 'botocore' the full wire trace | ||
will appear in your logs. If your payloads contain sensitive data this | ||
should not be used in production. |
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.
Nice touch! Glad this is in here.
administrators.rst
Outdated
|
||
Changing the log level for these entries from INFO to DEBUG will output the | ||
entire wire-trace between the Storage Service and your S3 implementation | ||
through the lens of the Boto3 SDK. The standard boto3 logging will provide |
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 seem to be capitalizing "Boto3" elsewhere - might be good to be consistent
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.
Good catch! It will be a blessing you won't have too many more PRs from me you have to pickup my terribly consistent habit of inconsistency!! 😆
Using some of the lessons learned with S3 recently we update the documentation to provide more tips on debugging than was available before.
1d2364a
to
e62162b
Compare
Merci buckets! One last thing on the list! Thanks Tessa 🙇 |
Using some of the lessons learned with S3 recently we update the
documentation to provide more tips on debugging than was available
before.
Connected to archivematica/Issues#1346