TrueCloudLab/frostfs-api
27
0
Fork 17
Code Issues 11 Pull requests 1 Releases Activity Actions

apemanager: Introduce proto-s for apemanager service #46

Merged
fyrchik merged 1 commit from aarifullin/frostfs-api:feat/apemanager into master 2024-04-27 13:48:05 +00:00
Conversation 0 Commits 1 Files changed 5 +554
aarifullin commented 2024-04-24 15:15:20 +00:00
Member
Copy link
  • Introduce proto-s for APEManagerService and related types.
  • Introduce a new status section related to APEManagerService.
  • Generate proto-docs.
* Introduce proto-s for `APEManagerService` and related types. * Introduce a new status section related to `APEManagerService`. * Generate proto-docs.
aarifullin force-pushed feat/apemanager from 102965060d to 1293e2092b 2024-04-24 15:15:52 +00:00 Compare
aarifullin requested review from storage-core-committers 2024-04-24 15:15:57 +00:00
aarifullin requested review from storage-core-developers 2024-04-24 15:15:57 +00:00
fyrchik requested review from realloc 2024-04-24 15:49:46 +00:00
fyrchik requested review from storage-services-committers 2024-04-24 15:49:46 +00:00
fyrchik requested review from storage-services-developers 2024-04-24 15:49:47 +00:00
acid-ant reviewed 2024-04-25 06:52:24 +00:00
apemanager/service.proto Outdated
@ -0,0 +149,4 @@
message ListChainsResponse {
message Body {
// The list of serialized rule chain.
repeated bytes chains = 1;
acid-ant commented 2024-04-25 06:52:24 +00:00
Member
Copy link

Why not to use type Chain here instead of bytes?

Why not to use type `Chain` here instead of `bytes`?
acid-ant commented 2024-04-25 06:53:45 +00:00
Member
Copy link

And the same approach for set\get.

And the same approach for `set\get`.
aarifullin commented 2024-04-25 07:13:17 +00:00
Author
Member
Copy link

Would you like to see Chain as alias for bytes?

Or you want this Chain see here? Let's consider what does it lead to:

  1. We are obliged to support Chain-s in frostfs-api/frostfs-api-go and Chain in policy-engine. Actually, frostfs-api needs to mirror structures in policy-engine
  2. We need converters from protobuf to policy-engine-s structures
Would you like to see `Chain` as alias for `bytes`? Or you want this [Chain](https://git.frostfs.info/TrueCloudLab/policy-engine/src/branch/master/pkg/chain/chain.go#L28-L34) see here? Let's consider what does it lead to: 1. We are obliged to support `Chain`-s in `frostfs-api/frostfs-api-go` and `Chain` in `policy-engine`. Actually, `frostfs-api` needs to mirror structures in `policy-engine` 2. We need converters from protobuf to `policy-engine`-s structures
acid-ant commented 2024-04-25 07:29:23 +00:00
Member
Copy link

Right, how about the same chain structure here as in policy-engine repo. Just want to know why bytes used here?

Right, how about the same `chain` structure here as in `policy-engine` repo. Just want to know why `bytes` used here?
aarifullin commented 2024-04-25 16:56:28 +00:00
Author
Member
Copy link

We agreed to keep []byte but as one of representation option

We agreed to keep `[]byte` but as one of representation option
acid-ant marked this conversation as resolved
aarifullin force-pushed feat/apemanager from 1293e2092b to 5e860e4997 2024-04-25 16:52:49 +00:00 Compare
dstepanov-yadro requested changes 2024-04-26 07:33:46 +00:00
apemanager/service.proto Outdated
@ -0,0 +1,172 @@
syntax = "proto3";
package neo.fs.v2.apemanager;
dstepanov-yadro commented 2024-04-26 07:26:04 +00:00
Member
Copy link

neo -> frostfs

neo -> frostfs
aarifullin commented 2024-04-26 16:44:03 +00:00
Author
Member
Copy link

Please, check

I renamed neo.fs.v2.apemanager to frostfs.v2.apemanager. I suppose we don't need to name it like frostfs.fs.v2.apemanager

Please, check I renamed `neo.fs.v2.apemanager` to `frostfs.v2.apemanager`. I suppose we don't need to name it like `frostfs.fs.v2.apemanager`
apemanager/service.proto Outdated
@ -0,0 +6,4 @@
import "session/types.proto";
option go_package = "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/apemanager/grpc;apemanager";
option csharp_namespace = "Neo.FileStorage.API.ApeManager";
dstepanov-yadro commented 2024-04-26 07:26:29 +00:00
Member
Copy link

Please drop this line. We don't use c# now.

Please drop this line. We don't use c# now.
aarifullin commented 2024-04-26 16:40:50 +00:00
Author
Member
Copy link

Fixed

Fixed
apemanager/types.proto Outdated
@ -0,0 +20,4 @@
// ChainTarget is an object to which a rule chain is defined.
message ChainTarget {
TargetType type = 1 [ json_name = "type" ];
dstepanov-yadro commented 2024-04-26 07:33:35 +00:00
Member
Copy link

Is this message special? Why does json_name defined?

Is this message special? Why does `json_name` defined?
aarifullin commented 2024-04-26 16:42:01 +00:00
Author
Member
Copy link

I removed json tags. I don't think they are really required

I removed json tags. I don't think they are really required
fyrchik commented 2024-04-27 07:03:55 +00:00
Owner
Copy link

We use them when we need to use camelCase (per some "best practice", probably from google, but I cannot find the link) instead of snake_case (protojson default).

We use them when we need to use `camelCase` (per some "best practice", probably from google, but I cannot find the link) instead of `snake_case` (protojson default).
aarifullin force-pushed feat/apemanager from 5e860e4997 to 39992c49fa 2024-04-26 16:40:20 +00:00 Compare
acid-ant approved these changes 2024-04-26 19:14:27 +00:00
dstepanov-yadro approved these changes 2024-04-27 13:47:42 +00:00
fyrchik merged commit c94b8ab6ae into master 2024-04-27 13:48:05 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
realloc
acid-ant
dstepanov-yadro
TrueCloudLab/storage-core-developers
TrueCloudLab/storage-services-committers
TrueCloudLab/storage-services-developers
Labels
Clear labels   
P0

Highest

  
P1

High

  
P2

Medium

  
P3

Low

  
good first issue

Good for newcomers

  
triage

Needs to be triaged

  
Infrastructure

Infrastructure, Automation, CI/CD or DevOps related stuff

  
blocked

The issue or PR is blocked by something

  
bug

Something is not working

  
config

Configuration format update or breaking change

  
discussion

Talking about something in order to reach a decision or to exchange ideas

  
documentation

Documentation related

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
go

Language features adoption

  
help wanted

Extra attention is needed

  
internal

   
invalid

Something is wrong

  
kludge

This is a kludge and we know it

  
observability

Related to metrics, tracing and logs

  
perfomance

Perfomance related

  
question

More information is needed

  
refactoring

Refactoring

  
wontfix

This won't be fixed

No labels
P0
P1
P2
P3
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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-api#46
No description provided.
Delete branch "aarifullin/frostfs-api:feat/apemanager"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?