support for sending commands to bot instances via PR comments #172

trz42 · 2023-04-06T19:20:39Z

This PR probably needs some updates/polishing but it has been tested already with a few PRs to the software-layer repository. It includes the following additions:

class to model action filters where an action is something like submitting a build job
- compared to setting a label bot:build an action can be filtered wrt a repository, an architecture and a bot instance name
- idea is that an action is only performed if its context (repository, architecture and bot instance) matches the filter
class to model bot commands which are composed of a command such as help and build and an action filter
adds code to parse NEW comments for bot commands (beginning at the start of a line with bot: ), displays if a command has been received, processes the command(s) and adds some result/output in the same comment
handlers for commands: help, build and showconfig
adds an app.cfg setting for limiting which GH accounts can send commands to the bot
refactors existing code to work well with the addition of action filters: one major change is how configuration settings are passed to where they are accessed
includes some polishing of docstrings and other things
last but not least, adds some unit tests for the action filters

~~TODOs~~ done

addressed the below by only running the critical code if the event is an issue_comment ... then the request_body has the key 'issue'
- clarify that update_pr_comment is working only with issue comment events (not with pull request events) because it accesses the issue section of an event, see line (in tools/pr_comments.py)
```
pr_number = int(request_body['issue']['number'])
```
- or change the code such that it works with both pull request events and issue comment events, e.g., by determining the type of the event

…comment updates

boegel · 2023-05-12T15:16:40Z

@trz42 Can you look into fixing the merge conflicts?

…eature/bot-rebuild

trz42 · 2023-05-12T16:57:46Z

@trz42 Can you look into fixing the merge conflicts?

Done.

boegel

first part of review, more coming soon...

eessi_bot_event_handler.py

tasks/build.py

boegel

Minor issue: tests are missing for stuff in tools/commands.py for example, but that's not a huge disaster (not worth blocking PR over imho)

boegel · 2023-05-26T10:58:05Z

eessi_bot_event_handler.py

+            return handler(event_info, bot_command)
+        else:
+            self.log(f"No handler for command '{cmd}'")
+            raise EESSIBotCommandError(f"unknown command '{cmd}'; use `bot: help` for usage information")


Maybe this should also result in a comment back to the issue/PR where the bot command was issued?

Unknown bot command 'foo', only valid bot commands are: ...

This can be done in a follow-up PR imho.

OK, will create an issue for that.

boegel · 2023-05-26T10:59:56Z

eessi_bot_event_handler.py

+        help_msg += "\n  - Commands must be sent with a **new** comment (edits of existing comments are ignored)."
+        help_msg += "\n  - A comment may contain multiple commands, one per line."
+        help_msg += "\n  - Every command begins at the start of a line and has the syntax `bot: COMMAND [ARGUMENTS]*`"
+        help_msg += "\n  - Currently supported COMMANDs are: `help`, `build`, `showconfig`"


This should eventually be replaced by a pointer to documentation (once it's there) - can be done in follow-up PR

eessi_bot_event_handler.py

tools/filter.py

tools/permissions.py

boegel · 2023-05-26T11:26:11Z

tools/permissions.py

+COMMAND_PERMISSION = "command_permission"
+
+
+def check_command_permission(account):


Checking of build and deploy permission should also come from here (eventually)?

yes makes sense

tools/pr_comments.py

eessi_bot_event_handler.py

boegel · 2023-06-05T13:54:57Z

eessi_bot_event_handler.py

+        #      ... in order to prevent surprises we should be careful what the bot
+        #      adds to comments, for example, before updating a comment it could
+        #      run the update through the function checking for a bot command.
+        if check_command_permission(sender) is False:


Since this is triggered for every issue comment being made, it's done too early, since for accounts that don't have permission to send commands to the bot this will immediately trigger a account XXX has NO permission to send commands to the bot (see boegel/software-layer#24 (comment)).

That should only be done if the comment effectively includes bot commands.

Let's try and make this the first thing we fix after merging this PR...

Opened #184 to follow up on this

boegel

I've tested this extensively with boegel/software-layer#24, and it's good to merge imho.

There's definitely room for improvement, but it's a nice step forward.

Thanks a lot for all the effort on this @trz42!

truib added 30 commits March 28, 2023 22:20

initial version of action filters plus some unit tests

759f75d

parse updates to PR comments for bot commands

0c7df9e

fixing errors

f04e622

add logging

6630509

improve logging

afa6560

more logging

17d22a4

change determining new part in comment

64c3d2f

allow for comment cleanup (old longer than new)

c949767

polish notification of command reception

c8dee13

polish more

39c7d42

log info if line does not contain a bot command

f73eaa3

also update PR comment if line does not contain a command

2702bf9

new module providing functions for managing permissions

0a2c678

add a safeguard to not let the bot enter endless loops in processing …

ff19606

…comment updates

make sure the bot does not act on its own comments (created/updated)

e2a5782

change logic in testing for command permissions

3d9cab1

drop updating if no bot command found

716b91f

use sender instead of author (owner) of a comment + some polishing

984dbd1

add missing f-string

931954f

define comment_new for created action

e72e6a1

add hint for where to add command

b44e298

only scan new comments for commands + list potential commands

7d13d7e

init bot command object from string

5988b60

take a string in constructor + add filter from string

3ec8b95

improve parsing of bot command

267a687

add update function for pr comments using event_info

ddf933f

fix data format and values in case of error or empty filters

d7aeecb

handling help command + smaller improvements

79e5b57

outlining how to apply filter

5517e33

changed event_dir to run_dir because value iss run_dir

95beb14

try to keep update to PR comment when command is 'help'

a794cfd

trz42 mentioned this pull request Apr 27, 2023

replay a GitHub event locally at a bot instance #178

Closed

Merge branch 'main' of gh-nessi:EESSI/eessi-bot-software-layer into f…

a3c305e

…eature/bot-rebuild

boegel requested changes May 26, 2023

View reviewed changes

tools/pr_comments.py Outdated Show resolved Hide resolved

eessi_bot_event_handler.py Show resolved Hide resolved

eessi_bot_event_handler.py Outdated Show resolved Hide resolved

eessi_bot_event_handler.py Show resolved Hide resolved

truib added 7 commits May 30, 2023 00:54

various updates to address review of PR172

236e326

use changed function name

d2c62e6

polish messages of bot + fixing minor issues

f8f19e4

improve formatting of results to bot commands

05c6243

adjust pytests to take changed return value into account

36851d1

use changed attribute name for comment id

13b7a27

IssueComment only has the attribute id, so renaming it in tests

de62337

poksumdo mentioned this pull request May 31, 2023

Test bot PR172 with current EESSI pilot (2021.12) trz42/software-layer#65

Open

truib added 4 commits May 31, 2023 07:02

also report to PR on GitHub if command sender has no permission

e6c527b

ensure that the bot does not comment on its own comments

1b42d77

adding information about configuring the feature to send commands

f189f6b

added check to only run update_pr_comment for issue_comment events

684820c

boegel reviewed Jun 5, 2023

View reviewed changes

boegel approved these changes Jun 5, 2023

View reviewed changes

boegel merged commit fb19a54 into EESSI:main Jun 5, 2023

boegel mentioned this pull request Jun 5, 2023

bot should only complain for comments that are actual bot commands #184

Open

This was referenced Jun 6, 2023

first outline of class to represent and manipulate pull request comments #163

Closed

bumping version to 2023.06 and masking OpenSSL to older than v3.* EESSI/compatibility-layer#184

Merged

This was referenced Jun 16, 2023

fix job manager crashes due to wrong value for pr_comment_id #185

Merged

job manager crash due to "Bad credentials" #142

Open

boegel added this to the 0.1.0 - initial release milestone Oct 4, 2023

boegel mentioned this pull request Oct 4, 2023

Allow conditional rebuilding on specific instances. #157

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

support for sending commands to bot instances via PR comments #172

support for sending commands to bot instances via PR comments #172

trz42 commented Apr 6, 2023 •

edited

Loading

boegel commented May 12, 2023

trz42 commented May 12, 2023

boegel left a comment

boegel left a comment

boegel May 26, 2023

trz42 May 29, 2023

boegel May 26, 2023

boegel May 26, 2023

trz42 May 29, 2023

boegel Jun 5, 2023

boegel Jun 5, 2023

boegel left a comment

		COMMAND_PERMISSION = "command_permission"


		def check_command_permission(account):

support for sending commands to bot instances via PR comments #172

support for sending commands to bot instances via PR comments #172

Conversation

trz42 commented Apr 6, 2023 • edited Loading

boegel commented May 12, 2023

trz42 commented May 12, 2023

boegel left a comment

Choose a reason for hiding this comment

boegel left a comment

Choose a reason for hiding this comment

boegel May 26, 2023

Choose a reason for hiding this comment

trz42 May 29, 2023

Choose a reason for hiding this comment

boegel May 26, 2023

Choose a reason for hiding this comment

boegel May 26, 2023

Choose a reason for hiding this comment

trz42 May 29, 2023

Choose a reason for hiding this comment

boegel Jun 5, 2023

Choose a reason for hiding this comment

boegel Jun 5, 2023

Choose a reason for hiding this comment

boegel left a comment

Choose a reason for hiding this comment

trz42 commented Apr 6, 2023 •

edited

Loading