Skip to content

Commit

Permalink
detect and filter out cancel events
Browse files Browse the repository at this point in the history
added end of line regex

fix one time check for cancel event

added test and fixed regex

test: add cache for 78492c2467876259d787538d600cfa0b18a2b814

slack: add culprit sha when notification can't be handled

fix

call generate for each webhook, defined expected output for new test

refactor if to match

extract filter logic out of generate_status_notification and into a function in generate_notifications in action.ml

removed vscode settings

added suprress_cancelle_events config value

set supress to true

moved function

refactor return blank list if filter fail instead of per webhook filter

use Re instead of Str

compile regexp before usage

removed .vscode and move to personal global

left new line at end of gitignore

removed .vscode

refactor to use Option.default, set default to true, added documentation

filter merge commit of main branch in status events

make test_promote

change filter to is, invert bool

github actions: update apt packages
  • Loading branch information
AHaliq committed Jun 29, 2020
1 parent 3ebeef2 commit d87c5e9
Show file tree
Hide file tree
Showing 13 changed files with 711 additions and 22 deletions.
1 change: 1 addition & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ jobs:

- run: opam pin add notabot.dev . --no-action

- run: sudo apt update
- run: opam depext notabot --yes
- run: opam depext notabot --yes --with-doc
- run: opam depext notabot --yes --with-test
Expand Down
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,12 @@ Some event notifications (e.g., status, commit comment) require a personal token

For more detailed instructions on token generation, refer to https://help.github.com/en/github/authenticating-to-github/creating-a-personal-access-token-for-the-command-line.

### Configuration values

| value | description | optional | default |
|-|-|-|-|
| `suppress_cancelled_events` | supresses status cancelled events | Yes | `true` |

## Testing (development)

```sh
Expand Down
28 changes: 22 additions & 6 deletions lib/action.ml
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@ let touching_label rule name =
(List.is_empty label_lc || List.mem ~equal:String.equal label_lc name_lc)
&& not (List.mem ~equal:String.equal ignore_lc name_lc)

let is_main_merge_message message n cfg =
let is_main_merge_message ~msg:message ~branch cfg =
match cfg.main_branch_name with
| Some main_branch ->
let branch = Github.get_commits_branch n in
let expect = sprintf "Merge branch '%s' into %s" main_branch branch in
let expect2 = sprintf "Merge remote-tracking branch 'origin/%s' into %s" main_branch branch in
let title = first_line message in
Expand Down Expand Up @@ -54,7 +53,8 @@ let partition_push cfg n =
n.commits
|> List.filter ~f:(fun c -> c.distinct)
|> List.filter ~f:(fun c ->
let skip = is_main_merge_message c.message n cfg in
let branch = Github.get_commits_branch n.ref in
let skip = is_main_merge_message ~msg:c.message ~branch cfg in
if skip then log#info "main branch merge, ignoring %s: %s" c.id (first_line c.message);
not skip)
|> List.map ~f:(fun commit ->
Expand Down Expand Up @@ -161,7 +161,14 @@ let partition_status cfg (n : status_notification) =
| None ->
let default = Option.value_map cfg.prefix_rules.default ~default:[] ~f:(fun webhook -> [ webhook ]) in
Lwt.return default
| Some commit -> Lwt.return (partition_commit cfg commit.files)
| Some commit ->
match
List.exists n.branches ~f:(fun { name } -> is_main_merge_message ~msg:commit.commit.message ~branch:name cfg)
with
| true ->
log#info "main branch merge, ignoring status event %s: %s" n.context (first_line commit.commit.message);
Lwt.return []
| false -> Lwt.return (partition_commit cfg commit.files)
in
match List.exists cfg.status_rules.status ~f:(Poly.equal n.state) with
| false -> Lwt.return []
Expand Down Expand Up @@ -189,6 +196,13 @@ let partition_commit_comment cfg n =
Lwt.return notifs
| l -> Lwt.return l

let is_cancelled_status_notification notification =
let { state; description; _ } = notification in
let r = Re.Str.regexp_case_fold "^\\(Build #[0-9]+ canceled by .+\\|Failed (exit status 255)\\)$" in
match description, state with
| Some s, Failure when Re.Str.string_match r s 0 -> true
| _ -> false

let generate_notifications cfg req =
match req with
| Github.Push n ->
Expand All @@ -214,8 +228,10 @@ let generate_notifications cfg req =
Lwt.return notifs
| Github.Status n ->
let%lwt webhooks = partition_status cfg n in
let notifs = List.map ~f:(fun webhook -> webhook, generate_status_notification n) webhooks in
Lwt.return notifs
( match is_cancelled_status_notification n && cfg.suppress_cancelled_events with
| true -> Lwt.return []
| _ -> Lwt.return (List.map ~f:(fun webhook -> webhook, generate_status_notification n) webhooks)
)
| _ -> Lwt.return []

let print_prefix_routing rules =
Expand Down
3 changes: 3 additions & 0 deletions lib/config.ml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type t = {
gh_token : string option;
offline : string option;
status_rules : status_rules;
suppress_cancelled_events : bool;
}

let make (json_config : Notabot_t.config) (secrets : Notabot_t.secrets) =
Expand Down Expand Up @@ -72,6 +73,7 @@ let make (json_config : Notabot_t.config) (secrets : Notabot_t.secrets) =
in
{ title = json_config.status_rules.title; status }
in
let suppress_cancelled_events = Option.default true json_config.suppress_cancelled_events in
{
chans;
prefix_rules = json_config.prefix_rules;
Expand All @@ -81,6 +83,7 @@ let make (json_config : Notabot_t.config) (secrets : Notabot_t.secrets) =
gh_token = secrets.gh_token;
offline = json_config.offline;
status_rules;
suppress_cancelled_events;
}

let load path secrets =
Expand Down
8 changes: 4 additions & 4 deletions lib/github.ml
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ let parse_exn ~secret headers body =
| ("member" | "create" | "delete" | "release") as event -> Event event
| event -> failwith @@ sprintf "unsupported event : %s" event

let get_commits_branch n =
match String.split ~on:'/' n.ref with
let get_commits_branch ref =
match String.split ~on:'/' ref with
| "refs" :: "heads" :: l -> String.concat ~sep:"/" l
| _ -> n.ref
| _ -> ref

let query_api ?token ~url parse =
let headers = Option.map token ~f:(fun t -> [ sprintf "Authorization: token %s" t ]) in
Expand All @@ -74,7 +74,7 @@ let generate_query_commmit cfg ~url ~sha =
let f = Caml.Filename.concat path sha in
( match Caml.Sys.file_exists f with
| false ->
log#error "unable to find offline file %s" f;
log#error "unable to find offline file %s, try fetching it from %s" f url;
Lwt.return_none
| true ->
Stdio.In_channel.with_file f ~f:(fun ic ->
Expand Down
1 change: 1 addition & 0 deletions lib/notabot.atd
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ type config = {
prefix_rules : prefix_config;
label_rules : label_config;
status_rules : status_config;
?suppress_cancelled_events : bool option; (* supresses status cancelled events; if not specified - true *)
?main_branch_name: string option; (* used to filter out notifications about merges of main branch into other branches *)
?offline: string option; (* where to find github api data when http calls are not allowed *)
}
Expand Down
28 changes: 18 additions & 10 deletions lib/slack.ml
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ let generate_pr_review_notification notification =
in
let summary =
Some
(sprintf "<%s|[%s]> %s <%s|%s> #%d <%s|%s>" repository.url repository.full_name sender.login
review.html_url action_str number html_url title)
(sprintf "<%s|[%s]> %s <%s|%s> #%d <%s|%s>" repository.url repository.full_name sender.login review.html_url
action_str number html_url title)
in
{
text = None;
Expand Down Expand Up @@ -126,8 +126,8 @@ let generate_pr_review_comment_notification notification =
in
let summary =
Some
(sprintf "<%s|[%s]> %s %s on #%d <%s|%s>" repository.url repository.full_name sender.login
action_str number html_url title)
(sprintf "<%s|[%s]> %s %s on #%d <%s|%s>" repository.url repository.full_name sender.login action_str number
html_url title)
in
let file =
match comment.path with
Expand Down Expand Up @@ -202,8 +202,8 @@ let generate_issue_comment_notification notification =
in
let summary =
Some
(sprintf "<%s|[%s]> %s <%s|%s> on #%d <%s|%s>" repository.url repository.full_name sender.login
comment.html_url action_str number issue.html_url title)
(sprintf "<%s|[%s]> %s <%s|%s> on #%d <%s|%s>" repository.url repository.full_name sender.login comment.html_url
action_str number issue.html_url title)
in
{
text = None;
Expand All @@ -226,7 +226,7 @@ let git_short_sha_hash hash = String.sub ~pos:0 ~len:8 hash

let generate_push_notification notification =
let { sender; created; deleted; forced; compare; commits; repository; _ } = notification in
let commits_branch = Github.get_commits_branch notification in
let commits_branch = Github.get_commits_branch notification.ref in
let tree_url = String.concat ~sep:"/" [ repository.url; "tree"; Uri.pct_encode commits_branch ] in
let title =
if deleted then
Expand Down Expand Up @@ -274,8 +274,8 @@ let generate_status_notification (notification : status_notification) =
| Error -> "error"
| _ ->
invalid_arg
(sprintf "Notabot doesn't know how to generate notification for the unexpected event %s"
(string_of_status_state state))
(sprintf "Notabot doesn't know how to generate notification for the unexpected event %s of %s"
(string_of_status_state state) sha)
in
let color_info =
match state with
Expand All @@ -287,7 +287,9 @@ let generate_status_notification (notification : status_notification) =
| None -> None
| Some s -> Some (sprintf "*Description*: %s." s)
in
let commit_info = sprintf "*Commit*: `<%s|%s>` %s - %s" html_url (git_short_sha_hash sha) (first_line message) author.login in
let commit_info =
sprintf "*Commit*: `<%s|%s>` %s - %s" html_url (git_short_sha_hash sha) (first_line message) author.login
in
let branches_info =
let branches = notification.branches |> List.map ~f:(fun ({ name } : branch) -> name) |> String.concat ~sep:", " in
sprintf "*Branches*: %s" branches
Expand All @@ -314,8 +316,14 @@ let generate_status_notification (notification : status_notification) =
pretext = summary;
color = Some color_info;
text = description_info;
<<<<<<< HEAD
fields = Some [ { title = None; value = String.concat ~sep:"\n" [ commit_info; branches_info ] } ];
||||||| parent of 72247fc... filter merge commit of main branch in status events
fields =
Some [ { title = None; value = String.concat ~sep:"\n" @@ [ commit_info; branches_info ] } ];
=======
fields = Some [ { title = None; value = String.concat ~sep:"\n" @@ [ commit_info; branches_info ] } ];
>>>>>>> 72247fc... filter merge commit of main branch in status events
};
];
blocks = None;
Expand Down
Loading

0 comments on commit d87c5e9

Please sign in to comment.