Skip to content

Do not use github.com/nspcc-dev/neofs-api-go/v2 where unnecessary#3057

Closed
cthulhu-rider wants to merge 21 commits intomasterfrom
avoid-apigo
Closed

Do not use github.com/nspcc-dev/neofs-api-go/v2 where unnecessary#3057
cthulhu-rider wants to merge 21 commits intomasterfrom
avoid-apigo

Conversation

@cthulhu-rider
Copy link
Contributor

@cthulhu-rider cthulhu-rider commented Dec 18, 2024 •
edited
Loading

self-reminder: check refs in commit messages before merging

a lot of changes, but this will maximally amortize coming update to the latest SDK

@codecov
Copy link

codecov bot commented Dec 18, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 11.16679% with 2482 lines in your changes missing coverage. Please review.

Project coverage is 22.92%. Comparing base (7ac9947) to head (fb1fd87).
Report is 61 commits behind head on master.

Files with missing lines Patch % Lines
pkg/services/object/server.go 10.23% 1339 Missing and 12 partials ⚠️
pkg/services/container/server.go 24.55% 240 Missing and 12 partials ⚠️
pkg/services/object/acl/v2/service.go 0.00% 158 Missing ⚠️
cmd/neofs-node/container.go 0.00% 104 Missing ⚠️
pkg/services/netmap/server.go 0.00% 95 Missing ⚠️
pkg/services/object/acl/eacl/v2/headers.go 19.48% 59 Missing and 3 partials ⚠️
cmd/neofs-node/reputation.go 0.00% 54 Missing ⚠️
pkg/services/session/server.go 0.00% 54 Missing ⚠️
pkg/services/accounting/server.go 0.00% 52 Missing ⚠️
cmd/neofs-node/object.go 0.00% 51 Missing ⚠️
... and 28 more
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3057      +/-   ##
==========================================
+ Coverage   22.36%   22.92%   +0.56%     
==========================================
  Files         793      747      -46     
  Lines       58713    57115    -1598     
==========================================
- Hits        13130    13093      -37     
+ Misses      44682    43134    -1548     
+ Partials      901      888      -13

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cthulhu-rider cthulhu-rider force-pushed the avoid-apigo branch 9 times, most recently from a369d5e to 9aab13c Compare December 25, 2024 09:49
@cthulhu-rider
Copy link
Contributor Author

cthulhu-rider commented Dec 25, 2024 •
edited
Loading

added temp commit "fixing" base branch to run tests manually. Results:

  1. container PASS
  2. object
FAILED pytest_tests/tests/object/test_object_api.py::TestObjectApi::test_object_parts_cannot_be_deleted - AssertionError: Regex pattern did not match.
FAILED pytest_tests/tests/object/test_object_lock.py::TestObjectLockWithGrpc::test_expired_object_should_be_deleted_after_locks_are_expired[complex object] - Failed: DID NOT RAISE <class 'Exception'>
  1. acl PASS
  2. failovers
FAILED pytest_tests/tests/failovers/test_failover_network.py::TestFailoverNetwork::test_block_storage_node_traffic - AttributeError: 'TimeoutExpired' object has no attribute 'strerror'
  1. contracts PASS
  2. load
FAILED pytest_tests/tests/load/test_load.py::TestLoad::test_custom_load - RuntimeError: Command: ../xk6-neofs/scenarios/preset/preset_grpc.py --size 1024  --containers 1 --out grpc.json --endpoint localhost:46765 --preload_obj 100 --policy "REP 2 IN X CBF 1 SELECT 2 FROM * AS X" --wallet ../xk6-neofs/xk6_wallet_jazdzmwfxq --confi...
============================================================================================================== 1 failed, 1 warning in 156.55s (0:02:36) ==

@cthulhu-rider
cli/control: Do not use API client from NeoFS SDK
7af817b 
It makes no sense to create client just get underlying connection from
it. Instead, direct gRPC dial should be used.

This inlines `Client.Dial` to keep previous behavior as much as possible
(some error texts could change only). URI parser is copy-pasted because
it's internalized by the SDK. Even if it'd export it, we are not ready
for the upgrade to the latest yet.

Starting from here, `cmd/neofs-cli` packages (all CLI apps actually) no
longer uses `github.com/nspcc-dev/neofs-api-go/v2` directly which is
good considering the upcoming abandonment of it.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
cli/control: Do not lose errors
933c9ab 
Regression from 9ac84e3. Detected by
the linter:
```
cmd/neofs-cli/modules/control/shards_dump.go:65:2    ineffassign  ineffectual assignment to err
cmd/neofs-cli/modules/control/synchronize_tree.go:41:6 ineffassign  ineffectual assignment to err
```

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
*: Avoid importing github.com/nspcc-dev/neofs-api-go/v2/rpc packages
7fa03a5 
This refactors storage node app making it to use gRPC connection to
remote nodes without wrappers. The behavior is preserved with some error
text added/changed for the better. The only place left will naturally
be gone with the next SDK upgrade.

One step closer to `github.com/nspcc-dev/neofs-api-go/v2` module
deprecation.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
storage/shard: Use imported constant from SDK lib, not api-go one
73d7ba0 
This was the only direct importer of to-be-deprecated `neofs-api-go/v2`
module `pkg/local_object_storage` space.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
node: Replace code from pkg/network/transport to pkg/services
f32e23a 
This will simplify subsequent refactoring with the abandonment of
`github.com/nspcc-dev/neofs-api-go/v2` module: gRPC handlers are
going to completely replace current `Server` interface definitions from
`pkg/services` space.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider cthulhu-rider force-pushed the avoid-apigo branch 5 times, most recently from 8850179 to 56c7d67 Compare December 27, 2024 12:06
@cthulhu-rider
pkg/services: Get rid of recurrent processing
f1589e4 
This inlines components previously defined as neofs-api-go `Server`
interfaces wrapping each other one-by-one into the gRPC server handlers.
The object service is big, and kept for now: it's going to be changed as
well soon.

In addition to reducing packages, components and code, this will make it
easier to deprecate `github.com/nspcc-dev/neofs-api-go/v2` module in the
near future.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
node/metrics: Refactor stat submitting in object ops
f58b461 
Use op enum to have one method for all ops. Submit success and duration
in one call.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
node/metrics: Make method used inside package only unexported
06db86d 
It's invisible outside anyway.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
services/object: Inline metric service
a48b7e8 
Continues b6fdbcc. This inlines one
level of recurrent object services into the gRPC server implementation.
The behavior is kept.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
services/object: Inline signing service
2e96580 
Continues f18a8ed. Now sign service is
gone.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
services/object: Refactor dependencies
e17ae1d 
Split the node abstraction into FS chain and local storage ones. The
service works only with these two root subsystems, so increasing the
number of abstractions is not expected, while the structure of the
real system is reflected better.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
services/object: Inline service setting response meta headers
ab7a2fb 
Continues f32a3b8. Now the service is
gone.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
services/object: Inline service checking node maintenance
c759e4f 
Continues a5902bb.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
service/object: Inline ACL service
8051803 
Continues 4d60cee.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
pkg/services/acl: Use constants from SDK
84fec64 
This will simplify neofs-api-go module deprecation.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
service/object: Inline transport splitting service
e412efc 
Continues 976e8b2.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
services/object: Inline intermediate DELETE service
5702d14 
Continues 4ed7f5f. Other ops are going
to be changed the same way in future commits.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
services/object: Inline intermediate SEARCH service
424f596 
Continues 6c09882.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
services/object: Inline intermediate PUT service
76b6f4a 
Continues f36289d.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
services/object: Inline intermediate GET/RANGE/HEAD services
8d89ebe 
Continues 068d746. All these ops are
very similar, so refactored in one scope.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
services/object: Inline intermediate HASH service
fb1fd87 
Continues 5d69dae. This completes
elimination of the recurrent architecture of object requests' handling.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
@cthulhu-rider
Copy link
Contributor Author

cthulhu-rider commented Dec 27, 2024

since the main branch is fixed, ive run tests here https://github.com/nspcc-dev/neofs-node/actions/runs/12515101826

@cthulhu-rider cthulhu-rider marked this pull request as ready for review December 27, 2024 13:38
@roman-khimov roman-khimov mentioned this pull request Dec 31, 2024
Open
roman-khimov
roman-khimov reviewed Dec 31, 2024
View reviewed changes
Copy link
Member

@roman-khimov roman-khimov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. I've looked at the first two patches, they're OK, please move them into a separate PR.
  2. This PR does much more than needed and it's a BAD thing. We need to go step by step with thing, api-go deprecation first and then all of the other things separately. Not that they're bad, they're mostly for good, but pushing it all into a single PR slows us down considerably. Real team work can't be effective in a setting where you have unexpected massive changes floating around.

@cthulhu-rider
Copy link
Contributor Author

cthulhu-rider commented Jan 9, 2025

ok, i'll split changes into several PRs

@cthulhu-rider cthulhu-rider marked this pull request as draft January 9, 2025 07:29
This was referenced Jan 9, 2025
Merged
Merged
Merged
Merged
@roman-khimov roman-khimov mentioned this pull request Jan 28, 2025
Merged
roman-khimov added a commit that referenced this pull request Jan 28, 2025
@roman-khimov
Rebuild object service processing pipeline (#3092)
b3e19e2 
This is yet another extraction from #3057, slightly adjusted. The most
controversial thing here is ACL change, but otherwise it tries to use
more of protobuf definitions and less of api-go definitions simplifying
the structure along the way.
@roman-khimov roman-khimov mentioned this pull request Jan 29, 2025
Merged
roman-khimov added a commit that referenced this pull request Jan 30, 2025
@roman-khimov
Object inlines (#3097)
9285b4f 
The last chunk from #3057. Less api-go overall, should be the same
functionally.
@roman-khimov
Copy link
Member

roman-khimov commented Jan 30, 2025

Merged in chunks.

@cthulhu-rider cthulhu-rider deleted the avoid-apigo branch March 17, 2025 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@roman-khimov roman-khimov roman-khimov left review comments

@carpawell carpawell Awaiting requested review from carpawell carpawell is a code owner

@End-rey End-rey Awaiting requested review from End-rey End-rey will be requested when the pull request is marked ready for review End-rey is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cthulhu-rider @roman-khimov