TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 105 Pull requests 8 Projects Releases 2 Activity Actions

ape: Make RemoveChainLocalOverride set correct removed flag #1006

Merged
fyrchik merged 1 commit from aarifullin/frostfs-node:fix/898_removed-true into master 2024-02-28 19:07:10 +00:00
Conversation 0 Commits 1 Files changed 5 +3 -14
5 changed files with 3 additions and 14 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
cmd/frostfs-cli/modules/control/remove_rule.go
View file

@ -57,11 +57,7 @@ func removeRule(cmd *cobra.Command, _ []string) {
verifyResponse(cmd, resp.GetSignature(), resp.GetBody()) verifyResponse(cmd, resp.GetSignature(), resp.GetBody())
if resp.GetBody().GetRemoved() { cmd.Println("Rule has been removed.")
cmd.Println("Rule has been removed.")
} else {
cmd.Println("Rule has not been removed.")
}
} }
func initControlRemoveRuleCmd() { func initControlRemoveRuleCmd() {

10
pkg/services/control/server/policy_engine.go
View file

@ -159,19 +159,13 @@ func (s *Server) RemoveChainLocalOverride(_ context.Context, req *control.Remove
return nil, err return nil, err
} }
removed := true
if err = s.localOverrideStorage.LocalStorage().RemoveOverride(apechain.Ingress, target, apechain.ID(req.GetBody().GetChainId())); err != nil { if err = s.localOverrideStorage.LocalStorage().RemoveOverride(apechain.Ingress, target, apechain.ID(req.GetBody().GetChainId())); err != nil {
code := getCodeByLocalStorageErr(err) if code := getCodeByLocalStorageErr(err); code != codes.NotFound {
if code == codes.NotFound {
removed = false
} else {
return nil, status.Error(code, err.Error()) return nil, status.Error(code, err.Error())
} }
} }
fyrchik commented 2024-02-26 12:17:09 +00:00
Outdated
Review
Copy link

So what do we need remove flag for if it is always true?

So what do we need `remove` flag for if it is always true?
aarifullin commented 2024-02-26 12:54:09 +00:00
Outdated
Review
Copy link

You're right. I have removed remove flag. But removed in the response body is left as a first implementation flavor, because we agreed about to return success even a chain is not found after control API had introduced Remove method

You're right. I have removed `remove` flag. But `removed` in the response body is left as a first implementation *flavor*, because we agreed about to return *success* even a chain is not found after control API had introduced `Remove` method
fyrchik commented 2024-02-26 16:34:14 +00:00
Outdated
Review
Copy link

I don't understand, what is the reason to have field in the body which is always true?
Is someone already dependent on it?

I don't understand, what is the reason to have field in the body which is always true? Is someone already dependent on it?
resp := &control.RemoveChainLocalOverrideResponse{ resp := &control.RemoveChainLocalOverrideResponse{
Body: &control.RemoveChainLocalOverrideResponse_Body{ Body: &control.RemoveChainLocalOverrideResponse_Body{},
Removed: removed,
},
} }
err = SignMessage(s.key, resp) err = SignMessage(s.key, resp)
fyrchik commented 2024-02-26 12:59:14 +00:00
Outdated
Review
Copy link

Do we need this in control service at all?

Do we need this in control service at all?
aarifullin commented 2024-02-27 09:31:10 +00:00
Outdated
Review
Copy link

Alright. It seems Removed is really confusing flag. I have removed it proto, regenerated protobufs and fix control API and cli!

Alright. It seems `Removed` is really confusing flag. I have removed it `proto`, regenerated protobufs and fix control API and cli!
if err != nil { if err != nil {

BIN
pkg/services/control/service.pb.go generated
View file

Binary file not shown.

1
pkg/services/control/service.proto
View file

@ -566,7 +566,6 @@ message RemoveChainLocalOverrideRequest {
message RemoveChainLocalOverrideResponse { message RemoveChainLocalOverrideResponse {
message Body { message Body {
bool removed = 1;
} }
Body body = 1; Body body = 1;

BIN
pkg/services/control/service_frostfs.pb.go generated
View file

Binary file not shown.