nspcc-dev / neofs-sdk-go

Go implementation of NeoFS SDK
Apache License 2.0
5 stars 14 forks source link

Drop `grpc`/`profobuff` dependencies #551

Open carpawell opened 5 months ago

carpawell commented 5 months ago

Is your feature request related to a problem? Please describe.

I'm always frustrated when I see them as explicit requirements in go.mod. Also, new ReplicateObject RPC is the only RPC that our API repo does not support (generated RPC only, no corresponding funcs in https://github.com/nspcc-dev/neofs-api-go/tree/70b1ffbd81414afce53290d61160b4716be02845/rpc), why?

Describe the solution you'd like

Do API-dependent things/implementations in the API repo, and reuse its functionality in SDK.

Describe alternatives you've considered

Nothing. Not sure I would think differently ever. At least -- if we change our perception of the API/SDK, their relations, etc -- we should do it once and with two repos at a time, not one by one.

Additional context

All the thoughts in the thread.

roman-khimov commented 5 months ago

I see your point and yes, inconsistency wrt api-go is bad. At the same time, how about dropping api-go? I don't see it solving any problem for us, there are zero other users of api-go, no one needs it except for SDK. You can argue about potential v3, but even that can be handled internally by SDK and other languages (all those 20 languages we support, right?) can do whatever they want with the stable API definitions.

carpawell commented 5 months ago

how about dropping api-go?

Dont mind. Maybe even a little "for". But what bothers me is that there is no issue about it (ok, I may be missing it but there is no link to it in the merged PR then), and it is "started" in some weird way with a single method that is still originally generated in api-go repo. Also, I do not like how often NeoFS does one thing and then does it back (SDK and api-go were together once, so much code was part of the node, then part of SDK, then there is an issue to make it part of the node again, etc). So much useless work (but api-go is full of useless wrappers itself, agree), we should discuss, make a decision, find a way to implement, and just do it, no partial unfinished work.

roman-khimov commented 5 months ago

there is no issue about it

Yes. We've not touched api-go for some time.

"started" in some weird way with a single method

Obviously, @cthulhu-rider just followed the path of least resistance which is to skip api-go. The choice was either to stop him doing that and lose some time refactoring the thing or accept this contribution that solves some problem as is (leaving the question for this issue, notice that we still can refactor, but after it's all integrated).

I do not like how often NeoFS does one thing and then does it back

Everyone hates it.

SDK and api-go were together once

They were separated to:

Both goals are good on their own, but over time you can notice that in fact:

The ideal world wrt this issue is:

But you know the reality, it doesn't and can't work this way for NeoFS. So we can keep api-go just because we have it and 10 years from now it might become useful in some way or we can forget about it and simplify the process.

there is an issue to make it part of the node again

Which one?

carpawell commented 5 months ago

Obviously, @cthulhu-rider just followed the path of least resistance which is to skip api-go.

I understand and deny this approach for things that are well structured and that a side reader can not expect. No issue about changing our approach (or no link to an issue), no discussion, just done and forgotten. Just like our replication pkg in the node.

api-go [...] we can forget about it and simplify the process

No problem, but where is an issue about? How to understand if we are dropping it or not?

there is an issue to make it part of the node again

Which one?

Just an example: https://github.com/nspcc-dev/neofs-sdk-go/pull/36 and https://github.com/nspcc-dev/neofs-sdk-go/pull/526, ok, it is not even an issue, it is a ready-to-merge suggestion (already done work, harder to complain at review stage, that is what i am talking about) that exactly declines the previous work. We did it more than once.

roman-khimov commented 5 months ago

How to understand if we are dropping it or not?

Communication. Like this one here.

Just an example: #36 and #526

We're not merging #526 in its current form for sure.