Add GetCurrentLock #4
Conversation
| result, err := l.dynamoDBAPI.GetItemWithContext(ctx, &getInput) | ||
|
|
||
| if err != nil { | ||
| l.logger.ErrorD("unexpected-dynamo-error", logger.M{ |
There was a problem hiding this comment.
nit: do we still want to log here since we're returning the error? If we do then maybe we also want to log for the unmarshal error below
There was a problem hiding this comment.
I think I was following the pattern that was established for other dynamo access - I'm not attached to this though.
and
Lines 186 to 189 in b4feadd
There was a problem hiding this comment.
I think when I made this library, my intention was to log somewhat verbosely and the user can control things by the logger argument to https://github.com/Clever/dynamodb-lock-go/blob/master/lock/interface.go#L75. The way things are done in this PR is consistent with that pattern, but I'm not super attached to the pattern.
lock/lock.go
Outdated
| if unmarshalErr := dynamodbattribute.UnmarshalMap(result.Item, &lock); unmarshalErr != nil { | ||
| return nil, fmt.Errorf("failed to DynamoDB unmarshal Record, %v", unmarshalErr) | ||
| } | ||
| if len(result.Item) == 0 { |
There was a problem hiding this comment.
Is there any reason to prefer doing this after attempting to unmarshal? Seems like it might be a teensy bit better to do it first?
GetCurrentLock will return the current lock for a key. This is meant to help troubleshoot lock starvation. If there is no lock, nil will be returned for both lock and error. This should not be used to "pre-check" for a lock as AcquireLock may still fail.
stevenjackson
force-pushed
the
getCurrentLock
branch
from
b4feadd
to
7cfe9db
Compare
Overview
GetCurrentLock will return the current lock for a key. This is meant to
help troubleshoot lock starvation. If there is no lock, nil will be
returned for both lock and error. This should not be used to "pre-check"
for a lock as AcquireLock may still fail.
Testing
2 new integration tests