Content store fix and import integration test#2696
Conversation
Move tar creation test utilities to separate package Test all supported formats for import Signed-off-by: Derek McGowan <[email protected]>
Signed-off-by: Derek McGowan <[email protected]>
|
The unit test failure found a real bug in the content store where an already exists error on commit would cause the content not get leased since the transaction would fail. Now the same error is returned but the transaction is allowed to complete with setting the lease when already exists is returned. This has a larger effect on import since the writer does not set the digest on open which correctly handles this condition. |
dmcgowan
changed the title
stevvooe
left a comment
stevvooe
left a comment
There was a problem hiding this comment.
LGTM
The only problem here is that the caller may assume the transaction was completely rolled back in the case of an error. However, I am not sure if we rely on that assumption.
estesp
left a comment
estesp
left a comment
There was a problem hiding this comment.
LGTM; do we need this content store fix cherry-picked to 1.1.x?
We don't document it but we don't guarantee that an already exists error didn't make a database change, the object certainly remains untouched but the lease is updated.
If we backport any of the fixes related to import yes, there was a bug fixed there which either caused this one or just didn't cover this case. Import is the only client operation I know of right now that will attempt to ingest to the content store without using a digest on open or commit. |
3 participants
Test all supported formats for import and a couple small fixes in the import.
Move tar creation test utilities to separate package.