TrueCloudLab/frostfs-api
32
0
Fork 18
Code Issues 14 Pull requests 2 Releases Activity Actions

Extend PatchRequest.Body with new_split_header field #81

Open
aarifullin wants to merge 1 commit from aarifullin/frostfs-api:fix/patch_split into master
pull from: aarifullin/frostfs-api:fix/patch_split
merge into: TrueCloudLab:master
Conversation 0 Commits 1 Files changed 1 +4
1 changed files with 4 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
object/service.proto
View file

@ -887,6 +887,10 @@ message PatchRequest {
// key, then it just replaces it while merging the lists.
bool replace_attributes = 3;
// New split header for the object. This defines how the object will relate
// to other objects in a split operation.
neo.fs.v2.object.Header.Split new_split_header = 5;
dkirillov marked this conversation as resolved
dkirillov commented 2025-03-05 06:20:57 +00:00
Review
Copy link

Do we decided add only split header, not any other system attributes?

Do we decided add only split header, not any other system attributes?
aarifullin commented 2025-03-05 08:24:40 +00:00
Review
Copy link

True, but I have changed my mind:

  1. I don't think these fields should become changeable for an object. Currently, we're trying to create a workaround to make an object related to other objects in frostfs. If we let a client patch attributes like ContainerID and OwnerID, then the result will be unpredictable (or it'll be a huge disaster)
  2. After our discussion I found that patching split_header is enough and no other system attribute is required to get patched. Initially my concern was to take into account all necessary fields
True, but I have changed my mind: 1. I don't think [these fields](https://git.frostfs.info/TrueCloudLab/frostfs-api/src/branch/master/object/types.proto#L80-L110) should become changeable for an object. Currently, we're trying to create a workaround to make an object related to other objects in frostfs. If we let a client patch attributes like `ContainerID` and `OwnerID`, then the result will be unpredictable (or it'll be a huge disaster) 2. After our discussion I found that patching `split_header` is enough and no other system attribute is required to get patched. Initially my concern was to take into account all necessary fields
fyrchik commented 2025-03-05 08:31:44 +00:00
Review
Copy link

I don't think these fields should become changeable for an object.

The same thing was being said about splitheader some time ago :)

>I don't think these fields should become changeable for an object. The same thing was being said about splitheader some time ago :)
😆 1
fyrchik commented 2025-03-05 08:33:19 +00:00
Review
Copy link

I don't mind adding this field explicitly, but we need to be sure that it is enough for the usecase we are trying to support.

I don't mind adding this field explicitly, but we need to be sure that it is enough for the usecase we are trying to support.
// The patch for the object's payload.
message Patch {
// The range of the source object for which the payload is replaced by the