Skip to content

[CLI] Add working-groups:verifyValidator#5057

Merged
mnaamani merged 5 commits intoJoystream:masterfrom
thesan:feature/cli-verify-validators
Jan 27, 2024
Merged

[CLI] Add working-groups:verifyValidator#5057
mnaamani merged 5 commits intoJoystream:masterfrom
thesan:feature/cli-verify-validators

Conversation

@thesan
Copy link
Copy Markdown
Collaborator

@thesan thesan commented Jan 23, 2024

Replace #4933

@zeeshanakram3

  • This can only be called by membership working group workers so regarding add CLI command verifyValidator #4933 (comment) I also enforced WorkingGroups.Membership but in a different way than add CLI command verifyValidator #4933. If you think it's a bad idea I can remove it. (Also for the same reason I didn't document the --group -g flags.
  • I updated the Readme but I'm not sure whether it's supposed to be automatized.
  • I tested the command on Atlas Dev it executed without error but the validator status wasn't set correctly true (but maybe the this network processor isn't up to date 🤷). I'll have to check locally.

Copy link
Copy Markdown
Contributor

@zeeshanakram3 zeeshanakram3 left a comment

Choose a reason for hiding this comment

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

I updated the Readme but I'm not sure whether it's supposed to be automatized.

Good idea. Maybe add this as part of build step?

Comment on lines +30 to +33
async init(): Promise<void> {
await super.init()
this._group = WorkingGroups.Membership
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

 ./bin/run working-groups:verifyValidator 2 
Initializing the query node connection (http://localhost:8081/graphql)...... done
Initializing the api connection (ws://localhost:9944)...... done
Current Group: storageProviders

Due to this change, whenever you run the command Current Group: storageProviders log will be printed, which might be confusing for the user

Also, instead of changing the group in code wouldn't it better to throw the error if user didn't specify the --group=membership flag (i.e. explicitly asking user to run command in membership group context)

Copy link
Copy Markdown
Contributor

@zeeshanakram3 zeeshanakram3 left a comment

Choose a reason for hiding this comment

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

LGTM

@mnaamani mnaamani merged commit f16df70 into Joystream:master Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants