Pseudo-transactionalize sharing #5029
Conversation
|
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
jessegeens
force-pushed
the
fix/tx-sharing
branch
from
ed00ee7 to
d159980
Compare
jessegeens
force-pushed
the
fix/tx-sharing
branch
from
d159980 to
ac1fc44
Compare
jessegeens
force-pushed
the
fix/tx-sharing
branch
from
ac1fc44 to
3d320c3
Compare
jessegeens
force-pushed
the
fix/tx-sharing
branch
2 times, most recently
from
a02cfef to
71aa634
Compare
jessegeens
force-pushed
the
fix/tx-sharing
branch
from
71aa634 to
8e2bf89
Compare
| if err != nil { | ||
| return nil, errors.Wrap(err, "gateway: error adding grant to storage") | ||
| } | ||
| if addGrantStatus.Code != rpc.Code_CODE_OK { |
There was a problem hiding this comment.
So following the discussion today, we could log here the addGrantStatus.Message before returning (or return it as part of the response, and then make sure it is logged), hoping to learn why the storage fails at times to create the share.
| return nil, errors.Wrap(err, "gateway: error calling CreateShare") | ||
| } | ||
| if res.Status.Code != rpc.Code_CODE_OK { | ||
| return res, nil |
There was a problem hiding this comment.
This seems some left-over copy/paste, probably to be dropped (we already return res, nil below anyway)
There was a problem hiding this comment.
Whoops, I missed this comment. If the RPC returns an error through res I think we should also return an error, so updated the code to do this
jessegeens
force-pushed
the
fix/tx-sharing
branch
from
8e2bf89 to
de19b5b
Compare
jessegeens
force-pushed
the
fix/tx-sharing
branch
from
de19b5b to
8da19df
Compare
Currently, sharing is not transactionalized: setting ACLs and writing the share to the db is completely independent. In the current situation, shares are written to the db before setting the ACL, letting users falsely believe that they successfully shared a resource, even if setting the ACL afterwards fails. his enhancement improves the situation by doing the least reliable (setting ACLs on EOS) first:
a) first pinging the db
b) writing the ACLs
c) writing to the db