TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 101 Pull requests 6 Projects Releases 1 Activity Actions

adm: Remove wallet requirement for safe methods #1179

Merged
fyrchik merged 2 commits from achuprov/frostfs-node:bugfix/remove-wallet-requirement-for-safe-methods into master 2024-07-11 14:04:44 +00:00
Conversation 3 Commits 2 Files changed 3 +87 -25
achuprov commented 2024-06-17 12:07:26 +00:00
Member
Copy link

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>

This PR changes the behavior of list-namespaces list-groups list-subjects list-group-subjects .

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> ``` This PR changes the behavior of `list-namespaces` `list-groups` `list-subjects` `list-group-subjects` .
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
Member
Copy link

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

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

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

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

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

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 Outdated
@ -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
Copy link

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

@achuprov

@achuprov
achuprov commented 2024-07-02 14:33:09 +00:00
Author
Member
Copy link

You are right. Fixed

You are right. Fixed
achuprov force-pushed bugfix/remove-wallet-requirement-for-safe-methods from bbedf0eaef to 12bec65465 2024-07-02 14:27:14 +00:00 Compare
achuprov force-pushed bugfix/remove-wallet-requirement-for-safe-methods from 12bec65465 to c9747545ab 2024-07-02 14:29:30 +00:00 Compare
achuprov force-pushed bugfix/remove-wallet-requirement-for-safe-methods from c9747545ab to 298281d78a 2024-07-03 04:06:42 +00:00 Compare
acid-ant reviewed 2024-07-03 07:32:45 +00:00
cmd/frostfs-adm/internal/modules/morph/frostfsid/frostfsid.go
@ -391,15 +413,22 @@ func frostfsidRemoveSubjectFromGroup(cmd *cobra.Command, _ []string) {
}
func frostfsidListGroupSubjects(cmd *cobra.Command, _ []string) {
acid-ant commented 2024-07-03 07:32:45 +00:00
Member
Copy link

Why these changes needed?

Why these changes needed?
achuprov commented 2024-07-03 09:05:22 +00:00
Author
Member
Copy link

fixed

fixed
acid-ant marked this conversation as resolved
fyrchik approved these changes 2024-07-03 08:01:13 +00:00
achuprov force-pushed bugfix/remove-wallet-requirement-for-safe-methods from 298281d78a to 554a865717 2024-07-03 09:04:43 +00:00 Compare
acid-ant approved these changes 2024-07-03 10:00:43 +00:00
achuprov force-pushed bugfix/remove-wallet-requirement-for-safe-methods from 554a865717 to a757cf2f57 2024-07-10 09:28:58 +00:00 Compare
fyrchik approved these changes 2024-07-10 09:51:12 +00:00
aarifullin commented 2024-07-10 11:25:44 +00:00
Member
Copy link

Please, check CI error messages

Please, check CI error messages
achuprov added 1 commit 2024-07-10 13:26:53 +00:00
[#1159] adm/frostfsid: Update frostfs-contract version
All checks were successful
DCO action / DCO (pull_request) Successful in 8m37s
Details
Vulncheck / Vulncheck (pull_request) Successful in 9m51s
Details
Build / Build Components (1.22) (pull_request) Successful in 10m46s
Details
Build / Build Components (1.21) (pull_request) Successful in 10m50s
Details
Tests and linters / gopls check (pull_request) Successful in 13m21s
Details
Tests and linters / Staticcheck (pull_request) Successful in 13m29s
Details
Tests and linters / Lint (pull_request) Successful in 13m55s
Details
Tests and linters / Tests (1.21) (pull_request) Successful in 19m10s
Details
Pre-commit hooks / Pre-commit (pull_request) Successful in 18m53s
Details
Tests and linters / Tests with -race (pull_request) Successful in 19m41s
Details
Tests and linters / Tests (1.22) (pull_request) Successful in 20m5s
Details
a090c023f8 
Signed-off-by: Alexander Chuprov <a.chuprov@yadro.com>
fyrchik approved these changes 2024-07-11 14:04:10 +00:00
fyrchik merged commit 0308835d48 into master 2024-07-11 14:04:44 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
acid-ant
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 labels
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
Projects
Clear projects
No items
No project
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
No description provided.
Delete branch "achuprov/frostfs-node:bugfix/remove-wallet-requirement-for-safe-methods"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?