TrueCloudLab/xk6-frostfs
42
1
Fork 20
Code Issues 33 Pull requests 1 Releases Activity Actions

Allow registry exporter to work with gRPC objects #142

Merged
fyrchik merged 1 commit from elebedeva/xk6-frostfs:fix/registry-export-grpc-obj into master 2024-06-04 12:17:23 +00:00
Conversation 0 Commits 1 Files changed 1 +45 -6
1 changed files with 45 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

49
internal/registry/obj_exporter.go
View file

@ -37,6 +37,7 @@ func (o *ObjExporter) ExportJSONPreGen(fileName string) error {
}
bucketMap := make(map[string]struct{})
containerMap := make(map[string]struct{})
count, err := o.selector.Count()
if err != nil {
@ -50,7 +51,7 @@ func (o *ObjExporter) ExportJSONPreGen(fileName string) error {
break
}
if _, err = f.WriteString(fmt.Sprintf(`%s{"bucket":"%s","object":"%s"}`, comma, info.S3Bucket, info.S3Key)); err != nil {
if err = writeObjectInfo(comma, info, f); err != nil {
return err
}
@ -58,15 +59,54 @@ func (o *ObjExporter) ExportJSONPreGen(fileName string) error {
comma = ","
}
if info.S3Bucket != "" {
bucketMap[info.S3Bucket] = struct{}{}
}
fyrchik marked this conversation as resolved Outdated
fyrchik commented 2024-05-31 18:48:00 +00:00
Outdated
Review
Copy link

This is wrong: len(bucketMap) can happily be 1 if we have exactly 1 S3 bucket, and in this case we need to print it.

What we might do instead is to have 2 maps (bucketMap and containerMap) and add only nonzero keys to them.

This is wrong: `len(bucketMap)` can happily be 1 if we have exactly 1 S3 bucket, and in this case we need to print it. What we might do instead is to have 2 maps (`bucketMap` and `containerMap`) and add only nonzero keys to them.
elebedeva commented 2024-06-03 20:18:11 +00:00
Outdated
Review
Copy link

fixed

fixed
if info.CID != "" {
containerMap[info.CID] = struct{}{}
}
}
acid-ant commented 2024-06-04 09:07:07 +00:00
Outdated
Review
Copy link

Can be shortened to:

if len(bucketMap) > 0 {
  err = writeContainerInfo("buckets", bucketMap, f)
} else {
  err = writeContainerInfo("containers", containerMap, f)
}
return err
Can be shortened to: ``` if len(bucketMap) > 0 { err = writeContainerInfo("buckets", bucketMap, f) } else { err = writeContainerInfo("containers", containerMap, f) } return err ```
elebedeva commented 2024-06-04 09:15:50 +00:00
Outdated
Review
Copy link

fixed

fixed
if _, err = f.WriteString(`],"buckets":[`); err != nil {
if _, err = f.WriteString(`]`); err != nil {
return err
fyrchik marked this conversation as resolved Outdated
fyrchik commented 2024-06-04 11:33:41 +00:00
Outdated
Review
Copy link

Why do you use errors.Join?
Error on write most likely will result in corrupted file (invalid JSON), we should just return the error.

Why do you use `errors.Join`? Error on write most likely will result in corrupted file (invalid JSON), we should just return the error.
elebedeva commented 2024-06-04 11:51:58 +00:00
Outdated
Review
Copy link

accidentally pushed, will fix

accidentally pushed, will fix
}
fyrchik marked this conversation as resolved Outdated
fyrchik commented 2024-06-04 11:05:03 +00:00
Outdated
Review
Copy link

This is tricky: each object was either put via gRPC or via S3.
But both can be in the registry at the same time, so we should print both maps, I believe.

This is tricky: each object was either put via gRPC or via S3. But both can be in the registry at the same time, so we should print both maps, I believe.
elebedeva commented 2024-06-04 12:07:09 +00:00
Outdated
Review
Copy link

fixed

fixed
if len(bucketMap) > 0 {
if err = writeContainerInfo("buckets", bucketMap, f); err != nil {
return err
}
}
if len(containerMap) > 0 {
if err = writeContainerInfo("containers", containerMap, f); err != nil {
return err
}
}
if _, err = f.WriteString(`}`); err != nil {
return err
}
fyrchik marked this conversation as resolved Outdated
fyrchik commented 2024-05-31 18:44:55 +00:00
Outdated
Review
Copy link

Why is it res += and not res = ?

Why is it `res += ` and not `res = `?
elebedeva commented 2024-06-03 20:19:13 +00:00
Outdated
Review
Copy link

Didn't notice, fixed

Didn't notice, fixed
return nil
}
func writeObjectInfo(comma string, info *ObjectInfo, f *os.File) (err error) {
var res string
if info.S3Bucket != "" || info.S3Key != "" {
res = fmt.Sprintf(`%s{"bucket":"%s","object":"%s"}`, comma, info.S3Bucket, info.S3Key)
fyrchik marked this conversation as resolved Outdated
fyrchik commented 2024-06-04 11:06:03 +00:00
Outdated
Review
Copy link

This change seems unnecessary and pollutes diff (it was bucketMap, we could continue to accept bucketMap).

This change seems unnecessary and pollutes diff (it was `bucketMap`, we could continue to accept `bucketMap`).
👍 1
} else {
res = fmt.Sprintf(`%s{"cid":"%s","oid":"%s"}`, comma, info.CID, info.OID)
}
_, err = f.WriteString(res)
return err
}
func writeContainerInfo(attrName string, bucketMap map[string]struct{}, f *os.File) (err error) {
if _, err = f.WriteString(fmt.Sprintf(`,"%s":[`, attrName)); err != nil {
return err
}
i := 0
comma = ""
comma := ""
for bucket := range bucketMap {
if _, err = f.WriteString(fmt.Sprintf(`%s"%s"`, comma, bucket)); err != nil {
return err
@ -76,7 +116,6 @@ func (o *ObjExporter) ExportJSONPreGen(fileName string) error {
}
i++
}
_, err = f.WriteString(`]}`)
_, err = f.WriteString(`]`)
return err
}