Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop grpc/profobuff dependencies #551

Open
carpawell opened this issue Jan 31, 2024 · 5 comments
Open

Drop grpc/profobuff dependencies #551

carpawell opened this issue Jan 31, 2024 · 5 comments
Labels
discussion Open discussion of some problem I4 No visible changes S1 Highly significant U4 Nothing urgent
Milestone

Comments

@carpawell
Copy link
Member

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.

@carpawell carpawell added U4 Nothing urgent S1 Highly significant I4 No visible changes labels Jan 31, 2024
@roman-khimov
Copy link
Member

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
Copy link
Member Author

carpawell commented Jan 31, 2024

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
Copy link
Member

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:

  • differentiate low-level API (they way it is in gRPC) and high-level API
  • simplify version transitions

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

  • low-level API is completely unusable, so the only user of api-go is sdk-go
  • some API separation can be easily achieved with packages (just look at the current pool, for example)
  • we still have a single version and they can be managed the other way
  • we lose some time on these boundaries each time we touch this area

The ideal world wrt this issue is:

  • you have protobuf (OpenAPI/anything else) API definition
  • you automatically generate bindings for your $LANGUAGE
  • you're good to go with these bindings

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?

@roman-khimov roman-khimov added the discussion Open discussion of some problem label Jan 31, 2024
@carpawell
Copy link
Member Author

carpawell commented Jan 31, 2024

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: #36 and #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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Open discussion of some problem I4 No visible changes S1 Highly significant U4 Nothing urgent
Projects
None yet
Development

No branches or pull requests

2 participants