Use containersOf in neofs-adm #200

fyrchik · 2023-04-03T11:47:37Z

fyrchik commented

2023-04-03 11:47:37 +00:00

Fix (partially) #175.

fyrchik added 1 commit 2023-04-03 11:47:38 +00:00

[#175 ] adm: list containers using containersOf 6a7155fc90

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>

acid-ant approved these changes 2023-04-03 13:22:42 +00:00

dstepanov-yadro requested changes 2023-04-03 14:20:18 +00:00

cmd/frostfs-adm/internal/modules/morph/container.go Outdated

					
				@ -117,3 +125,3 @@

					}

					out, err := json.Marshal(containers)

					_, err = f.Write([]byte{']'})

dstepanov-yadro commented

2023-04-03 14:20:11 +00:00

If written = 0, then only one parenthesis will go to the file?

fyrchik commented

2023-04-03 15:53:53 +00:00

Fixed

fyrchik marked this conversation as resolved

fyrchik force-pushed fix-containers-of from 7294708168 to 6e693335e1

2023-04-03 15:53:54 +00:00

Compare

dstepanov-yadro approved these changes 2023-04-03 16:58:35 +00:00

aarifullin reviewed 2023-04-03 21:59:37 +00:00

cmd/frostfs-adm/internal/modules/morph/container.go

					
				@ -113,1 +115,3 @@

							cnt.EACL = ea

						// Writing directly to the file is ok, because json.Encoder does no internal buffering.

						if written != 0 {

							_, err = f.Write([]byte{','})

aarifullin commented

2023-04-03 21:59:37 +00:00

Just for my curiosity.
Are cnt-s big? It they are not, wouldn't be better to collect them in one slice?

var cnts []Container

and then invoke data, err := json.Marshal(&cnts) and just ioutil.WriteFile() for once?

If they are big, then yes - stream marshalling is not supported by encoding/json and the such way is needed

Just for my curiosity. Are `cnt`-s big? It they are not, wouldn't be better to collect them in one slice? ``` var cnts []Container ``` and then invoke `data, err := json.Marshal(&cnts)` and just `ioutil.WriteFile()` for once? If they are big, then yes - stream marshalling is not supported by `encoding/json` and the such way is needed

fyrchik commented

2023-04-04 06:43:18 +00:00

It was done like this before.
We have no limits on the number of containers: there can be a lot of them and eACL can be big too (64kb is the limit, already happened in practice).

It was done like this before. We have no limits on the number of containers: there can be a lot of them and `eACL` can be big too (64kb is the limit, already happened in practice).

👍 1

carpawell reviewed 2023-04-04 07:21:27 +00:00

cmd/frostfs-adm/internal/modules/morph/container.go

					
				@ -50,2 +49,2 @@

					if _, ok := itm.(stackitem.Null); !ok {

						return unwrap.ArrayOfBytes(res, err)

					// Nothing bad, except live session on the server, do not report to the user.

					defer func() { _ = inv.TerminateSession(sid) }()

carpawell commented

2023-04-03 15:19:23 +00:00

not sure exactly, but not that necessary as i remember according to my new neo-go API discoverings (4 months ago, could be wrong), actors do not do that as i remember (again, could be wrong)

fyrchik commented

2023-04-04 07:22:37 +00:00

That is why we ignore error.

carpawell commented

2023-04-04 07:23:29 +00:00

I meant that it looks like neo-go team also thinks that we can drop defer at all. But not a problem.

I meant that it looks like `neo-go` team also thinks that we can drop `defer` at all. But not a problem.

fyrchik commented

2023-04-04 07:24:58 +00:00

It costs us (almost) nothing and neo-go internals can change any time.

carpawell marked this conversation as resolved

cmd/frostfs-adm/internal/modules/morph/container.go

					
				@ -120,0 +133,4 @@

				func dumpSingleContainer(bw *io.BufBinWriter, ch util.Uint160, inv *invoker.Invoker, id []byte) (*Container, error) {

					bw.Reset()

					emit.AppCall(bw.BinWriter, ch, "get", callflag.All, id)