fix/expired-locked-object-replication #138

carpawell · 2023-03-15T01:08:48Z

carpawell commented

2023-03-15 01:08:48 +00:00

Closes #67.

carpawell requested review from storage-core-developers 2023-03-15 01:08:56 +00:00

carpawell self-assigned this 2023-03-15 01:09:07 +00:00

carpawell reviewed 2023-03-15 01:10:53 +00:00

pkg/core/object/fmt.go Outdated

													
				@ -323,0 +338,4 @@

						locked, err := v.e.IsLocked(addr)

						if err != nil {

							return fmt.Errorf("locking status check: %w", err)

carpawell commented

2023-03-15 01:10:53 +00:00

Poster

i can imagine that we could store an object anyway in that branch but not sure. @TrueCloudLab/storage-core-committers

fyrchik commented

2023-03-15 05:22:53 +00:00

If there was some storage error, it would probably better not to store anything.
Also, I would somehow mention the word expired in the error message (can't check locking status for an expired object?)

If there was some storage error, it would probably better not to store anything. Also, I would somehow mention the word `expired` in the error message (`can't check locking status for an expired object`?)

carpawell commented

2023-03-16 13:22:31 +00:00

Poster

added

fyrchik approved these changes 2023-03-15 05:23:25 +00:00

acid-ant approved these changes 2023-03-15 05:57:07 +00:00

dstepanov-yadro reviewed 2023-03-15 14:16:43 +00:00

pkg/core/object/fmt.go Outdated

													
				@ -39,2 +40,4 @@

				}

				// LockSource is a source of lock relations between the objects.

				type LockSource interface {

dstepanov-yadro commented

2023-03-15 14:16:36 +00:00

Do we really need to make this interface exportable and write comments?

carpawell commented

2023-03-16 13:26:15 +00:00

Poster

we usually do such in our packages since it should be implemented in another package

in fact i would use just StorageEngine here but cycled imports appeared so i decided to do that as usual

cc @fyrchik

we usually do such in our packages since it should be implemented in another package in fact i would use just `StorageEngine` here but cycled imports appeared so i decided to do that as usual cc @fyrchik

fyrchik commented

2023-03-16 14:47:41 +00:00

We use it as an argument to the exported option, seems ok to me.

fyrchik marked this conversation as resolved

carpawell added the due date 2023-03-17

2023-03-15 15:49:34 +00:00

carpawell force-pushed fix/expired-locked-object-replication from 281bd7df23 to 64bde68fb9

2023-03-16 13:22:30 +00:00

Compare

fyrchik merged commit 64bde68fb9 into master

2023-03-16 14:47:54 +00:00

carpawell deleted branch fix/expired-locked-object-replication