search

instructlab / instructlab-bot

GitHub bot to assist with the taxonomy contribution workflow
Apache License 2.0
13 stars 16 forks source link

Can we remove the `bot enable` step and instead allow any taxonomy triagers to run bot commands? #253

Closed ckadner closed 3 months ago

ckadner commented 3 months ago

Even after the bot is enabled, only taxonomy triagers should be allowed to run bot commands so we should be checking permissions for all commands anyways?

https://github.com/instructlab/instruct-lab-bot/blob/d0a4506ae58a1374cebb245341994cc6b03138fe/gobot/handlers/issue_comment.go#L314

@russellb @nerdalert

vishnoianil commented 3 months ago

@ckadner When we implemented the access control, the idea was that anybody can run the precheck/generate, once triager marks the PR good for testing (basically "enable"). So currently anybody can launch the precheck once enable is done.

If we want to restrict to only traigers to run precheck/generate, we don't need "enable", but it will restrict the PR author itself to run the prechec/generate on it. We can implemented this if we would like to go that path.

ckadner commented 3 months ago

My thoughts are, that once we go public, it might be prudent to be more restrictive in who can run the bot and how often, given that it can result in significant usage of compute resources.

And, although informative to contributors, most of the information produced by the bot is intended to facilitate the triaging process.

vishnoianil commented 3 months ago

@ckadner Sounds good to me. I will push the patch sometime tomorrow.

russellb commented 3 months ago

I have no problem with the change to make the process smoother for triagers right now.

And, although informative to contributors, most of the information produced by the bot is intended to facilitate the triaging process.

FWIW, the original inspiration for starting this bot was to help contributors. We watched countless folks struggle in the pilot without access to hardware capable of doing even the basic stuff in ilab. I do think serving contributors to help them not put the entirety of testing on triagers is a worthwhile goal, but it doesn't have to be solved right now.

vishnoianil commented 3 months ago

I have no problem with the change to make the process smoother for triagers right now.

And, although informative to contributors, most of the information produced by the bot is intended to facilitate the triaging process.

FWIW, the original inspiration for starting this bot was to help contributors. We watched countless folks struggle in the pilot without access to hardware capable of doing even the basic stuff in ilab. I do think serving contributors to help them not put the entirety of testing on triagers is a worthwhile goal, but it doesn't have to be solved right now.

Agree. We are implementing it in a way, that we can remove this restriction by just a change the config file and restarting the bot. So if we decide to allow contributors to run the checks, we can do it without needing any code change. This is one of the guideline that we are keeping in our mind when implementing these functions.