[SS-68] iceberg/aws: add more detailed error logging/context#35183
Conversation
|
Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone. PR title guidelines
Pre-merge checklist
|
DAlperin
changed the title
ublubu
left a comment
ublubu
left a comment
There was a problem hiding this comment.
LGTM
One comment inline, looking for a doc comment or name change to clarify how important auth_method()'s output is.
| } | ||
|
|
||
| impl AwsConnection { | ||
| pub(crate) fn auth_method(&self) -> &'static str { |
There was a problem hiding this comment.
Is this auth_method string representation somehow canonical? Is it just for display? Is it a key for searching logs?
There was a problem hiding this comment.
just added to make log searching easy. not super attached to the strings themselves, happy to change
There was a problem hiding this comment.
I meant that it's not obvious to me, from reading auth_method, what I should feel comfortable using the strings for.
Something like debug_auth_method or a comment on the function.
There is this really weird intermittent failure in prod where it fails to load credentials with http timeout. Cleaning up the context so we know where it might come from is helpful.
DAlperin
force-pushed
the
dov/iceberg-aws-detailed-error-context
branch
from
65d6fd5 to
dd20aa8
Compare
2 participants
There is this really weird intermittent failure in prod where it fails to load credentials with http timeout. Cleaning up the context so we know where it might come from is helpful.