Skip to content
This repository was archived by the owner on Jun 30, 2022. It is now read-only.

Conversation

@enthusiastmartin
Copy link

Hi,

In HydraDX, we liked the bench-bot and wanted to use it in similar way it is done in substrate/polkadot repository.

However , it needed some adjustments so it could be used with our Substrate project.

And instead of just hacking the bench-bot to fit our project - we thought to make changes in a way that the bench-bot can be used with any other substrate project which follows standard Substrate template node repository.

It might be beneficial for other substrate projects.

So within this PR, we added handling of other than substrate and polkadot repositories.

Currently, it is all done separately to keep the benchmarking process for substrate/polkadot intact as it is slightly differs in few things. And it should fit any project which uses substrate template node as a template.

It is actually still work in progress ( hence the draft pr) - I am still adding few improvements.

And I also want to add some features like whitelisted users and cancel benchmarks - which I saw too in Bench-bot v2 spec created by @shawntabrizi ( but that might be done separately ).

But i thought why not just to create PR to propose and ask you for opinions.

We also needed to run the benchmarks on specific machine , so there is also implemented a possibility to run all commands on a remote machine by specifying remote host/user.

@shawntabrizi
Copy link
Member

@enthusiastmartin hey, i just saw this for the first time.

Is this something you are continuing to work on?

Does your changes work as is?

@enthusiastmartin
Copy link
Author

enthusiastmartin commented Mar 1, 2021

@enthusiastmartin hey, i just saw this for the first time.

Is this something you are continuing to work on?

Does your changes work as is?

Yes, as this PR is - it works.

We made few other modifications for our purposes, but that's just our needs.

I have a plan to add some features which I had mentioned ( mainly the whitelist of users and cancel ) soon. So yes, still planning to continue some work on this.

Would you be happy with such features as well ?

@shawntabrizi
Copy link
Member

@enthusiastmartin sorry for delayed responses here.

I am very happy to merge any of these additional features in. Please let me know when it is ready to review and I will bring it in!

@enthusiastmartin enthusiastmartin marked this pull request as ready for review April 19, 2021 08:50
@cla-bot-2021
Copy link

cla-bot-2021 bot commented May 11, 2021

User @enthusiastmartin, please sign the CLA here.

@joao-paulo-parity
Copy link
Contributor

Hi @enthusiastmartin, I'll be taking over maintenance for this bot from now on

I saw that you split the modules and reorganized some of the code as part of your changes. I don't like the idea of building upon the existing code as it currently is really hard to maintain and reason about. Some of your changes fit in the notion of a "refactor" which is what I plan to do soon, but not with the code base in its current state. So for a starting point I'll try to merge your PRs (#10 and #11) into a "develop" branch and start reworking from there.

Once this rework is done I'll reference your PRs in the new PR I'll be opening.

@enthusiastmartin
Copy link
Author

Hi @enthusiastmartin, I'll be taking over maintenance for this bot from now on

I saw that you split the modules and reorganized some of the code as part of your changes. I don't like the idea of building upon the existing code as it currently is really hard to maintain and reason about. Some of your changes fit in the notion of a "refactor" which is what I plan to do soon, but not with the code base in its current state. So for a starting point I'll try to merge your PRs (#10 and #11) into a "develop" branch and start reworking from there.

Once this rework is done I'll reference your PRs in the new PR I'll be opening.

Ok, great. fine with with me of course.

yes, i kind of agree. i added this work on top of what was currently there but i did not want to touch and change the implementation for substtrate/polkadot repo as mentioned in my PR description.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants