TrueCloudLab
/
frostfs-node
18
1
Fork 20
Code Issues 88 Pull Requests 7 Actions Releases Activity

adm: Remove wallet requirement for safe methods #1179

Open
achuprov wants to merge 2 commits from achuprov/frostfs-node:bugfix/remove-wallet-requirement-for-safe-methods into master
pull from: achuprov/frostfs-node:bugfix/remove-wallet-requirement-for-safe-methods
merge into: TrueCloudLab:master
Conversation 2 Commits 2 Files Changed 3
achuprov commented 2024-06-17 12:07:26 +00:00
Collaborator

Close #1159

Before

frostfs-adm morph frostfsid list-namespaces -r localhost:40332
init contract invoker: can't to initialize context: can't read alphabet wallets dir: open : no such file or directory

After

 ./bin/frostfs-adm morph frostfsid list-namespaces -r localhost:40332
<root>
Close #1159 Before ``` frostfs-adm morph frostfsid list-namespaces -r localhost:40332 init contract invoker: can't to initialize context: can't read alphabet wallets dir: open : no such file or directory ``` After ``` ./bin/frostfs-adm morph frostfsid list-namespaces -r localhost:40332 <root> ```
achuprov added 3 commits 2024-06-17 12:07:31 +00:00
6f455e9b5f [#1159] adm: Remove wallet requirement for list-groups 
Signed-off-by: Alexander Chuprov <a.chuprov@yadro.com>
3ea8c58bf1 [#1159] adm: Remove wallet requirement for list-subject 
Signed-off-by: Alexander Chuprov <a.chuprov@yadro.com>
DCO action / DCO (pull_request) Successful in 2m0s Details
Build / Build Components (1.21) (pull_request) Failing after 2m49s Details
Build / Build Components (1.22) (pull_request) Failing after 2m51s Details
Vulncheck / Vulncheck (pull_request) Failing after 3m59s Details
Tests and linters / Lint (pull_request) Failing after 4m49s Details
Tests and linters / Staticcheck (pull_request) Failing after 4m39s Details
Tests and linters / gopls check (pull_request) Failing after 5m20s Details
Tests and linters / Tests (1.21) (pull_request) Failing after 7m57s Details
Tests and linters / Tests (1.22) (pull_request) Failing after 8m3s Details
Tests and linters / Tests with -race (pull_request) Failing after 8m0s Details
Pre-commit hooks / Pre-commit (pull_request) Successful in 9m17s Details
902c0c8ab5 [#1159] adm: Remove wallet requirement for list-namespace 
Signed-off-by: Alexander Chuprov <a.chuprov@yadro.com>
achuprov force-pushed bugfix/remove-wallet-requirement-for-safe-methods from 902c0c8ab5 to 9cc1f94360 2024-06-18 15:25:27 +00:00 Compare
achuprov changed title from WIP: bugfix/remove-wallet-requirement-for-safe-methods to remove wallet requirement for safe methods 2024-06-18 16:11:39 +00:00
achuprov requested review from storage-core-committers 2024-06-18 16:11:52 +00:00
achuprov requested review from storage-core-developers 2024-06-18 16:11:54 +00:00
aarifullin commented 2024-06-19 13:10:42 +00:00
Collaborator

As you have exported util functions from frostfs-contract, please, could you refactor morph client for frostfsid contract here: https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/morph/client/frostfsid/subject.go?

As you have exported util functions from `frostfs-contract`, please, could you refactor morph client for frostfsid contract here: [https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/morph/client/frostfsid/subject.go](https://git.frostfs.info/TrueCloudLab/frostfs-node/src/branch/master/pkg/morph/client/frostfsid/subject.go)?
acid-ant reviewed 2024-06-20 07:53:22 +00:00
cmd/frostfs-adm/internal/modules/morph/frostfsid/frostfsid.go Outdated
@ -252,3 +262,2 @@
func frostfsidListNamespaces(cmd *cobra.Command, _ []string) {
ffsid, err := newFrostfsIDClient(cmd)
commonCmd.ExitOnErr(cmd, "init contract invoker: %w", err)
c, err := helper.GetN3Client(viper.GetViper())
acid-ant commented 2024-06-20 07:46:05 +00:00
Collaborator

Why not to move this code to the separate function? All you need in frostfsidListNamespaces(...) is a result from commonclient.ReadIteratorItems(...).
Looks like you can reuse it in frostfsidListSubjects, frostfsidListGroups, frostfsidListGroupSubjects.

Why not to move this code to the separate function? All you need in `frostfsidListNamespaces(...)` is a result from `commonclient.ReadIteratorItems(...)`. Looks like you can reuse it in `frostfsidListSubjects`, `frostfsidListGroups`, `frostfsidListGroupSubjects`.
acid-ant changed title from remove wallet requirement for safe methods to adm: Remove wallet requirement for safe methods 2024-06-20 07:53:47 +00:00
fyrchik changed title from adm: Remove wallet requirement for safe methods to WIP: adm: Remove wallet requirement for safe methods 2024-06-20 11:28:36 +00:00
achuprov force-pushed bugfix/remove-wallet-requirement-for-safe-methods from 9cc1f94360 to 0eb0d1d0fe 2024-06-21 14:27:35 +00:00 Compare
achuprov force-pushed bugfix/remove-wallet-requirement-for-safe-methods from 0eb0d1d0fe to bd38778c1d 2024-06-21 14:37:05 +00:00 Compare
achuprov changed title from WIP: adm: Remove wallet requirement for safe methods to adm: Remove wallet requirement for safe methods 2024-06-21 14:38:29 +00:00
acid-ant reviewed 2024-06-21 16:23:28 +00:00
cmd/frostfs-adm/internal/modules/morph/frostfsid/frostfsid.go Outdated
@ -391,28 +401,29 @@ func frostfsidRemoveSubjectFromGroup(cmd *cobra.Command, _ []string) {
}
func frostfsidListGroupSubjects(cmd *cobra.Command, _ []string) {
c, err := helper.GetN3Client(viper.GetViper())
acid-ant commented 2024-06-21 16:23:28 +00:00
Collaborator

Looks like here we can use initInvoker() too.

Looks like here we can use `initInvoker()` too.
fyrchik commented 2024-06-22 03:14:33 +00:00
Owner

could you refactor morph client for frostfsid contract here

Could you create a task for this? Would like to merge this PR ASAP.

>could you refactor morph client for frostfsid contract here Could you create a task for this? Would like to merge this PR ASAP.
👍 1
fyrchik reviewed 2024-06-24 06:41:04 +00:00
cmd/frostfs-adm/internal/modules/morph/frostfsid/frostfsid.go Outdated
@ -414,2 +425,2 @@
cmd.Printf("%s (%s)\n", address.Uint160ToString(subjAddr), subj.Name)
for _, group := range groups {
cmd.Printf("%s (%d)\n", group.Name, group.ID)
fyrchik commented 2024-06-24 06:41:04 +00:00
Owner

The diff seems too large: previously this method used ListGroupSubjects, now it uses ListGroups, why so?

The diff seems too large: previously this method used `ListGroupSubjects`, now it uses `ListGroups`, why so?
fyrchik reviewed 2024-06-24 06:41:08 +00:00
cmd/frostfs-adm/internal/modules/morph/frostfsid/frostfsid.go Outdated
@ -414,2 +425,2 @@
cmd.Printf("%s (%s)\n", address.Uint160ToString(subjAddr), subj.Name)
for _, group := range groups {
cmd.Printf("%s (%d)\n", group.Name, group.ID)
fyrchik commented 2024-06-24 06:41:08 +00:00
Owner

The diff seems too large: previously this method used ListGroupSubjects, now it uses ListGroups, why so?

The diff seems too large: previously this method used `ListGroupSubjects`, now it uses `ListGroups`, why so?
achuprov force-pushed bugfix/remove-wallet-requirement-for-safe-methods from bd38778c1d to bbedf0eaef 2024-06-24 11:50:13 +00:00 Compare
fyrchik requested changes 2024-06-24 13:15:07 +00:00
cmd/frostfs-adm/internal/modules/morph/frostfsid/frostfsid.go
@ -315,3 +321,2 @@
subj, err := ffsid.roCli.GetSubject(addr)
commonCmd.ExitOnErr(cmd, "get subject: %w", err)
items, err := unwrap.Array(inv.Call(nmHash, getSubjectContract, addr))
fyrchik commented 2024-06-24 13:15:05 +00:00
Owner

Autogenerated RPC bindings for the frostfsID have GetSubject (which is almost exactly the same as your unwrap.Array(...)), why not use those?

Autogenerated RPC bindings for the `frostfsID` have `GetSubject` (which is almost exactly the same as your `unwrap.Array(...)`), why not use those?
fyrchik commented 2024-07-01 10:20:37 +00:00
Owner

@achuprov

@achuprov
All checks were successful
Vulncheck / Vulncheck (pull_request) Successful in 3m34s
Details
Tests and linters / Staticcheck (pull_request) Successful in 4m13s
Details
Build / Build Components (1.21) (pull_request) Successful in 4m21s
Details
Build / Build Components (1.22) (pull_request) Successful in 4m17s
Details
Tests and linters / gopls check (pull_request) Successful in 4m44s
Details
Pre-commit hooks / Pre-commit (pull_request) Successful in 5m23s
Details
Tests and linters / Lint (pull_request) Successful in 5m27s
Details
Tests and linters / Tests (1.21) (pull_request) Successful in 9m37s
Details
Tests and linters / Tests with -race (pull_request) Successful in 9m47s
Details
Tests and linters / Tests (1.22) (pull_request) Successful in 10m8s
Details
DCO action / DCO (pull_request) Successful in 1m4s
Details
This Pull Request doesn't have enough approvals yet. 0 of 2 approvals granted.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
Sign in to join this conversation.
Reviewers
No reviewers
fyrchik
TrueCloudLab/storage-core-developers
Labels
Clear labels   
P0

Highest   
P1

High   
P2

Medium   
P3

Low   
badger

Badger DB related issues   
frostfs-adm
   
frostfs-cli
   
frostfs-ir
   
frostfs-lens
   
frostfs-node
   
good first issue

Good for newcomers   
triage
  
Infrastructure

Infrastructure, Automation, CI/CD or DevOps related stuff   
blocked

The issue or PR is blocked by something   
bug

Something is not working   
config

Configuration format update or breaking change   
discussion

Talking about something in order to reach a decision or to exchange ideas   
documentation

Documentation related   
duplicate

This issue or pull request already exists   
enhancement

New feature   
go

Language features adoption   
help wanted

Extra attention is needed   
internal
   
invalid

Something is wrong   
kludge

This is a kludge and we know it   
observability

Related to metrics, tracing and logs   
perfomance

Perfomance related   
question

More information is needed   
refactoring

Refactoring   
wontfix

This won't be fixed
No Label
P0
P1
P2
P3
badger
frostfs-adm
frostfs-cli
frostfs-ir
frostfs-lens
frostfs-node
good first issue
triage
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#1179
There is no content yet.
Delete Branch "achuprov/frostfs-node:bugfix/remove-wallet-requirement-for-safe-methods"

Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?