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

Revise internal/neofs package #494

Open
cthulhu-rider opened this issue Jun 3, 2022 · 1 comment
Open

Revise internal/neofs package #494

cthulhu-rider opened this issue Jun 3, 2022 · 1 comment
Labels
enhancement Improving existing functionality good first issue Good for newcomers I4 No visible changes S2 Regular significance U4 Nothing urgent

Comments

@cthulhu-rider
Copy link
Contributor

Initial idea of ​​creating a type

type NeoFS struct {
was to have a mediator between NeoFS system and all S3 needs.

After subsequent refactors its orientation has been changed to providing application-specific features. I suggest to revise this architecture.

I can think of two possible approaches

internal/neofs - global window into NeoFS

We get rid of any app-specific info from internal/neofs package. It imports only NeoFS SDK Go functionality. NeoFS type provides full functionality required within the framework of S3 needs, but can be used as an autonomous entitiy (true mediator). Each application implements its NeoFS-related needs locally through single internal/neofs.NeoFS type. Seems like

app_1 <=== local dependency (multi-interface struct) <=== internal/neofs.NeoFS
app_2 <=== local dependency (multi-interface struct) <=== internal/neofs.NeoFS

Some local dependencies can be similar, but the are not the same.

internal/neofs - collection of implementations

Differs with previous approach that each type in the internal/neofs package is an implementation of one dependency of one application. The mediator mentioned in the first approach is also present, but it is not exported from the package. Seems like

app_1 <=== local dependency (interface) <=== internal/neofs.NeoFS_App1 (multi-interface struct) [private] <=== internal/neofs.neofs]
app_2 <=== local dependency (interface) <=== internal/neofs.NeoFS_App2 (multi-interface struct) [private] <=== internal/neofs.neofs]

As for me, first approach looks clearer. In both approaches, I consider it important to loosen the coupling of types between applications.

@cthulhu-rider cthulhu-rider added good first issue Good for newcomers proposal discussion Open discussion of some problem labels Jun 3, 2022
@cthulhu-rider
Copy link
Contributor Author

cthulhu-rider commented Jun 3, 2022

Future proposal

We can go further and develop internal/neofs layer into a full-fledged intermediate layer between NeoFS (SDK actually) and S3 needs. I find it extremely useful for the following reasons:

  • everything that S3 services need from NeoFS at a glance
  • simple and controlled update of the SDK version
  • accumulation of practices of using SDK, which can be incorporated into proposals and even supported natively
  • almost zero performance overhead: type wrappers doesn't require any extra runtime memory and exist only in source code

Cons: it takes time to implement in the current code base. But if the proposal comes in, I can contribute to the implementation.

@roman-khimov roman-khimov added enhancement Improving existing functionality U4 Nothing urgent S2 Regular significance I4 No visible changes and removed proposal discussion Open discussion of some problem labels Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving existing functionality good first issue Good for newcomers I4 No visible changes S2 Regular significance U4 Nothing urgent
Projects
None yet
Development

No branches or pull requests

2 participants