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

cmd actions bench & fmt #406

Conversation

mordamax
Copy link
Contributor

@mordamax mordamax commented Jul 26, 2024

Closes #128
Closes https://github.com/paritytech/infrastructure/issues/41

Adds ability to run some standard helper commands, for now only for benchmarks and fmt (with taplo)
It will commit the result of operation back into PR with some reports and/or links to logs

bench uses gitrun-001 runner which corresponds to Validator Reference Hardware

  • Does not require a CHANGELOG entry
Examples (screenshots):

Benchmarks

Google Chrome 2024-07-26 13 33 05

Bench results with Subweight report

Google Chrome 2024-07-26 13 35 36

/cmd [command] --clean - removing bot's and author's command runs to clean up PR

Google Chrome 2024-07-26 13 33 56

/cmd [command] --help - outputs the usage instructions

Google Chrome 2024-07-26 13 33 25

.github/scripts/cmd/cmd.py Outdated Show resolved Hide resolved
Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

Good work, thanks! I mostly skimmed it but i think this is one of the things where we just need to experiment a bit.

Only point is that there should be a commands.md or bot.md with some overview of how the bot works, what commands are available and how the components make it work.
The weight-generation file seems to be specific already.

mordamax and others added 2 commits July 31, 2024 23:40
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@github-actions github-actions bot requested a review from ggwpez July 31, 2024 22:42
Copy link

Review required! Latest push from author must always be reviewed

1 similar comment
Copy link

Review required! Latest push from author must always be reviewed

Copy link
Contributor

@xlc xlc left a comment

Choose a reason for hiding this comment

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

best to be able to only allow fellowship members to trigger the bot to prevent abuses especially with self hosted runners


jobs:
acknowledge:
if: ${{ startsWith(github.event.comment.body, '/cmd') }}
Copy link
Contributor

Choose a reason for hiding this comment

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

anyone can trigger this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep good point, will add a similar check as for /merge
and cmd.md guide
thanks

@xlc
Copy link
Contributor

xlc commented Jul 31, 2024

I don't know what's the plan for the self hosted runners but for ORML we are using github webhook to create new AWS EC2 instance upon request and autoscale it down when idle. In that way we don't need to run idle instance and able to scale up as many runners as needed.

CMD: ${{ steps.get-pr-comment.outputs.group2 }} # to avoid "" around the command
run: |
echo 'help<<EOF' >> $GITHUB_OUTPUT
python3 .github/scripts/cmd/cmd.py $CMD >> $GITHUB_OUTPUT

Choose a reason for hiding this comment

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

This line doesn't look safe, I'll write in Element.

Copy link
Contributor Author

@mordamax mordamax Aug 1, 2024

Choose a reason for hiding this comment

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

Improved regex which prevents passing arbitrary commands + check for fellows

image image

@mordamax mordamax force-pushed the mak-testing-benchmarks-2 branch 2 times, most recently from fbdf2d6 to 0bed466 Compare August 1, 2024 15:40
@kianenigma
Copy link
Contributor

While this is being prepared, what is the standard way to generate weight files? I suppose I need access to a 'standard machine'?

Asking for #364

@mordamax
Copy link
Contributor Author

mordamax commented Aug 2, 2024

While this is being prepared, what is the standard way to generate weight files? I suppose I need access to a 'standard machine'?

Asking for #364

if it waits until early next week, i'd recommend to wait a bit. @tugytur was going to spawn a runner as soon as @bkchr provides details, hope today or early next week 🤞

If not, then you can follow this guidance https://github.com/polkadot-fellows/runtimes/blob/main/docs/weight-generation.md and find standard machine with similar specs

@kianenigma
Copy link
Contributor

Not super rushed from my side, but thanks for the context :)

@github-actions github-actions bot requested a review from ggwpez August 18, 2024 21:52
@mordamax
Copy link
Contributor Author

/merge

@fellowship-merge-bot
Copy link
Contributor

Enabled auto-merge in Pull Request

Available commands
  • /merge: Enables auto-merge for Pull Request
  • /merge cancel: Cancels auto-merge for Pull Request
  • /merge help: Shows this menu

For more information see the documentation

@fellowship-merge-bot fellowship-merge-bot bot enabled auto-merge (squash) August 26, 2024 11:42
@fellowship-merge-bot fellowship-merge-bot bot merged commit 16d2db7 into polkadot-fellows:main Aug 26, 2024
46 of 47 checks passed
@ggwpez ggwpez deleted the mak-testing-benchmarks-2 branch August 26, 2024 13:20
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.

Script to automate weight generation
6 participants