Add PutSingle implementation #486

Merged
fyrchik merged 1 commit from dstepanov-yadro/frostfs-node:feat/put_single into master 2023-07-26 21:07:59 +00:00
Relates https://git.frostfs.info/TrueCloudLab/frostfs-api/issues/9
dstepanov-yadro force-pushed feat/put_single from d74ed24e08 to fe7228d27d 2023-07-03 08:39:57 +00:00 Compare
dstepanov-yadro force-pushed feat/put_single from afe6628551 to c01c96de52 2023-07-04 12:30:28 +00:00 Compare
dstepanov-yadro force-pushed feat/put_single from c01c96de52 to 23e09e6897 2023-07-04 12:31:33 +00:00 Compare
dstepanov-yadro force-pushed feat/put_single from 23e09e6897 to 191a91b938 2023-07-04 12:55:51 +00:00 Compare
dstepanov-yadro force-pushed feat/put_single from 191a91b938 to aff38c4322 2023-07-05 08:44:30 +00:00 Compare
dstepanov-yadro force-pushed feat/put_single from aff38c4322 to 6da225b09a 2023-07-07 08:35:17 +00:00 Compare
dstepanov-yadro requested review from storage-core-committers 2023-07-07 08:35:38 +00:00
dstepanov-yadro requested review from storage-core-developers 2023-07-07 08:35:39 +00:00
dstepanov-yadro changed title from WIP: Add PutSingle implementation to Add PutSingle implementation 2023-07-07 08:35:45 +00:00
dstepanov-yadro added 1 commit 2023-07-07 14:48:21 +00:00
[#482] Enable staticcheck in foregjo actions
All checks were successful
Tests and linters / Tests (1.19) (pull_request) Successful in 3m28s
Tests and linters / Lint (pull_request) Successful in 4m16s
ci/woodpecker/pr/pre-commit Pipeline was successful
Build / Build Components (1.19) (pull_request) Successful in 4m1s
Tests and linters / Staticcheck (pull_request) Successful in 5m25s
Build / Build Components (1.20) (pull_request) Successful in 2m23s
Tests and linters / Tests (1.20) (pull_request) Successful in 15m48s
Tests and linters / Tests with -race (pull_request) Successful in 22m25s
ci/woodpecker/push/pre-commit Pipeline was successful
a0d51090a4
Signed-off-by: Anton Nikiforov <an.nikiforov@yadro.com>
dstepanov-yadro force-pushed feat/put_single from bc23500c1f to 362978edb4 2023-07-10 06:59:52 +00:00 Compare
aarifullin approved these changes 2023-07-10 07:54:54 +00:00
aarifullin left a comment
Member

LGTM

LGTM
fyrchik approved these changes 2023-07-10 10:21:59 +00:00
ale64bit approved these changes 2023-07-10 12:30:39 +00:00
@ -446,0 +494,4 @@
if !b.checker.CheckBasicACL(reqInfo) || !b.checker.StickyBitCheck(reqInfo, idOwner) {
return nil, basicACLErr(reqInfo)
} else if err := b.checker.CheckEACL(request, reqInfo); err != nil {
Member

nit: the else seems unnecessary.

nit: the `else` seems unnecessary.
Author
Member

fixed

fixed
ale64bit marked this conversation as resolved
@ -0,0 +34,4 @@
type putSingleRequestSigner struct {
req *objectAPI.PutSingleRequest
keyStorage *svcutil.KeyStorage
signer *sync.Once
Member

the pointer is not needed. Optionally, you can just embed the sync.Once and call Do directly.

the pointer is not needed. Optionally, you can just embed the `sync.Once` and call `Do` directly.
Author
Member

I prefer pointers in fields, so I'll leave it that way if it's not a bug.

I prefer pointers in fields, so I'll leave it that way if it's not a bug.
Member

I prefer pointers in fields, so I'll leave it that way if it's not a bug.

it's not a bug, but it makes it easier to forget filling the field and a panic will follow. Without pointer is not without disadvantages (e.g. it can be accidentally copied), but still. Up to you.

> I prefer pointers in fields, so I'll leave it that way if it's not a bug. it's not a bug, but it makes it easier to forget filling the field and a panic will follow. Without pointer is not without disadvantages (e.g. it can be accidentally copied), but still. Up to you.
ale64bit marked this conversation as resolved
@ -0,0 +43,4 @@
metaHdr := new(sessionV2.RequestMetaHeader)
meta := s.req.GetMetaHeader()
metaHdr.SetTTL(meta.GetTTL() - 1)
Member

why minus one?

why minus one?
Author
Member

because current node handled request, so for the next node TTL = current TTL - 1

because current node handled request, so for the next node TTL = current TTL - 1
@ -0,0 +131,4 @@
func (s *Service) validatePutSingleObject(ctx context.Context, obj *objectSDK.Object) error {
if err := s.fmtValidator.Validate(ctx, obj, false); err != nil {
return fmt.Errorf("coult not validate object format: %w", err)
Member

s/coult/could

s/coult/could
Author
Member

fixed

fixed
ale64bit marked this conversation as resolved
@ -0,0 +204,4 @@
return fmt.Errorf("could not create object placement traverser: %w", err)
}
resultError := &atomic.Value{}
Member

doesn't need to be pointer. Pass it via &resultError to saveToPlacementNodes.

doesn't need to be pointer. Pass it via `&resultError` to `saveToPlacementNodes`.
Author
Member

fixed

fixed
ale64bit marked this conversation as resolved
@ -0,0 +240,4 @@
nodeAddresses []placement.Node,
resultError *atomic.Value,
) bool {
wg := &sync.WaitGroup{}
Member

doesn't need to be pointer.

doesn't need to be pointer.
Author
Member

fixed

fixed
ale64bit marked this conversation as resolved
@ -0,0 +248,4 @@
continue
}
wg.Add(1)
Member

can we move this to the line right before Submit? makes it easier to read if wg.Add and wg.Done are close to each other.

can we move this to the line right before `Submit`? makes it easier to read if `wg.Add` and `wg.Done` are close to each other.
Author
Member

fixed

fixed
ale64bit marked this conversation as resolved
@ -0,0 +336,4 @@
)
}
stop = err == nil
Member

nit: these are a bit hard to follow, but not sure how to improve it.

nit: these are a bit hard to follow, but not sure how to improve it.
ale64bit marked this conversation as resolved
dstepanov-yadro force-pushed feat/put_single from 362978edb4 to f412c18a4e 2023-07-10 12:34:57 +00:00 Compare
dstepanov-yadro force-pushed feat/put_single from f412c18a4e to 909de9c193 2023-07-10 12:38:12 +00:00 Compare
dstepanov-yadro force-pushed feat/put_single from 337f6f7c23 to b9623a09df 2023-07-10 12:47:17 +00:00 Compare
dstepanov-yadro force-pushed feat/put_single from b9623a09df to a65e26878b 2023-07-10 12:49:46 +00:00 Compare
fyrchik merged commit a65e26878b into master 2023-07-10 14:36:35 +00:00
fyrchik referenced this pull request from a commit 2023-07-10 14:36:35 +00:00
fyrchik referenced this pull request from a commit 2023-07-10 14:36:35 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
4 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#486
No description provided.