apertium / phenny

This is a port of phenny, a Python IRC bot, to Python3. This specific version, called begiak, is a fork of the Wadsworth version, used by the apertium project.
http://wiki.apertium.org/wiki/Begiak
Other
16 stars 42 forks source link

solution to many repeat commit messages (and the like) #407

Open jonorthwash opened 6 years ago

jonorthwash commented 6 years ago

Instead of getting 400 distinct messages when a bot does a bunch of commits (or similar), we should somehow group them into a single message.

jonorthwash commented 6 years ago

I see two possible main approaches:

  1. Queue up messages and either
    • report a summary every minute or something,
    • detect similar commits (over some period of time?) and report a single message for similar ones.
  2. Stop reporting (and say "..."?) if more than n (2?) commits in a given time period (a minute or so?).
Androbin commented 6 years ago

The problem is that we don't want to accidentally swallow messages.

jonorthwash commented 6 years ago

How might that happen?

And anyway, missing a few messages would be better than flooding the channel as we have now.

Androbin commented 6 years ago

There are messages we want to see. There are messages we don't want to see.

If we want to see them, we shouldn't hide them. If we don't want to see them, we shouldn't generate them.

We can certainly collapse large chunks. Do you have some examples at hand for how to do summaries?

sushain97 commented 6 years ago

Do you have some examples at hand for how to do summaries?

See my comment about debounce in #408.

jonorthwash commented 6 years ago

Which of the options I presented is debounce? And which is throttling?

sushain97 commented 6 years ago

report a summary every minute or something,

This is throttling.

Stop reporting (and say "..."?) if more than n (2?) commits in a given time period (a minute or so?).

This is effectively debouncing but not exactly.

jonorthwash commented 6 years ago

So what exactly is debouncing?

sushain97 commented 6 years ago

Debouncing will bunch a series of sequential calls to a function into a single call to that function. It ensures that one notification is made for an event that fires multiple times.

"implementation"-wise:

Throttle: the original function be called at most once per specified period. Debounce: the original function be called after the caller stops calling the decorated function after a specified period.

from https://stackoverflow.com/questions/25991367/difference-between-throttling-and-debouncing-a-function

Androbin commented 6 years ago

For clarification: Is debouncing semantically equivalent to Android Bundled Notifications? Notifications with equal tag can sign up to be collapsed into a single (expandable) notification.

sushain97 commented 6 years ago

Similar but it's not really debouncing since you mutate the existing notification, not hold on to notifications and then display the summary version after you don't get a new one for a while.

On Mar 18, 2018 6:14 AM, "Robin Richtsfeld" notifications@github.com wrote:

For clarification: Is debouncing semantically equivalent to Android Bundled Notifications? Notifications with equal tag can sign up to be collapsed into a single (expandable) notification.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/apertium/phenny/issues/407#issuecomment-373989882, or mute the thread https://github.com/notifications/unsubscribe-auth/AEBEfuDsMMp8Dm0mNw7SEkqNdeAhOEzLks5tfkGMgaJpZM4Su5Cg .

jonorthwash commented 6 years ago

So the current hack I implemented does the following for multiple commit messages in a single push:

(11:08:13) begiak: ud-annotatrix: keggsmurph21 * src/test/data/plain-text.js: add data from issue #292 https://github.com/jonorthwash/ud-annotatrix/commit/2447b5, ud-annotatrix: keggsmurph21 * src/test/detect.js: flip order of the data iterator and it() call (so we know exactly which case we fail on) https://github.com/jonorthwash/ud-annotatrix/commit/4f938f, ud-annotatrix: keggsmurph21 * src/detect.js: change regex to determine format for
(11:08:17) begiak: CoNLL-U for only looking at non-word (/\W/) to specifically look for space (/\s/) chars https://github.com/jonorthwash/ud-annotatrix/commit/99a1dd, ud-annotatrix: keggsmurph21 * public/js/bundle.js: latest build https://github.com/jonorthwash/ud-annotatrix/commit/dc7d83

@ftyers, what do you think of this behaviour? How might the above example ideally look?

ftyers commented 6 years ago

It's still a bit wordy... although better. There is no need to repeat the username and as I mentioned in previous conversations I don't think we need to repeat the URL every time. It should be enough to give just the commit hashes, e.g.

(11:08:13) begiak: ud-annotatrix: keggsmurph21 [2447b5 4f938f 99a1dd dc7d83] * add data from issue #292 * flip order of the data iterator and it() call (so we know exactly which case we fail on) * change regex to determine format for CoNLL-U for only looking at non-word (/\W/) to specifically look for space (/\s/) chars * latest build 

Then we could have a begiak plugin to give a url to a commit e.g. !commit 4f938f.

jonorthwash commented 6 years ago

Then we could have a begiak plugin to give a url to a commit e.g. !commit 4f938f.

How do you propose that it find the correct repo for that commit hash?

sushain97 commented 6 years ago

Should be able to just hit the GH search API:

e.g. https://github.com/search?q=674c1cfdf645ac6cafe4be3277e20d567a43b23d+org%3Aapertium&type=Commits

Androbin commented 6 years ago

curl -H "Accept: application/vnd.github.cloak-preview" https://api.github.com/search/commits?q=674c1cfdf645ac6cafe4be3277e20d567a43b23d

Androbin commented 6 years ago

I have prepared the summary format as described by @ftyers and the !commit command in #422.

jonorthwash commented 6 years ago

@Androbin, I merged your PR, and can't seem to get .commit to work right:

(19:14:58) begiak: apertium-bak: zu-ann [ 4f4581c ] apertium-bak.bak.lexc: new adj
(19:15:49) begiak: apertium-tat: zu-ann [ 204b2c9 ] apertium-tat.tat.lexc: new adj
...
(22:28:09) firespeaker: .commit 204b2c9
(22:28:10) begiak: firespeaker: lots and lots of changes
(22:28:26) firespeaker: .commit 4f4581c
(22:28:26) begiak: firespeaker: FOR REAL.
jonorthwash commented 6 years ago

Now I'm getting "problematic payloads" too—i.e., payloads that begiak can't handle.

Specifically there seems to be an error about lack of key "sha"; from the logs:

ERROR: Error 501 (commits were https://github.com/apertium/apertium-sah/commit/ea8ace4f0d80bde40b165637c4aecc68d8ab6cf9, https://github.
com/apertium/apertium-sah/commit/e7a9ae0550d099fe8b801ca0dcc4f2e9b9620654)
ERROR: {'before': '413214eb27381627e4a7d59db236c4277da5c4d7', 'after': 'e7a9ae0550d099fe8b801ca0dcc4f2e9b9620654', 'forced': False, 'com
mits': [{'committer': {'name': 'jonorthwash', 'email': 'jonathan.north.washington@gmail.com', 'username': 'jonorthwash'}, 'url': 'https:
//github.com/apertium/apertium-sah/commit/ea8ace4f0d80bde40b165637c4aecc68d8ab6cf9', 'added': [], 'author': {'name': 'jonorthwash', 'ema
il': 'jonathan.north.washington@gmail.com', 'username': 'jonorthwash'}, 'modified': ['apertium-sah.sah.lexc', 'tests/passives.yaml'], 't
ree_id': '7239a3f34cb58bc1262173c0ef6b987d11aa0e05', 'removed': [], 'id': 'ea8ace4f0d80bde40b165637c4aecc68d8ab6cf9', 'timestamp': '2018-07-22T22:46:54-04:00', 'message': 'some stuff from the bible, passives', 'distinct': True}, {'committer': {'name': 'jonorthwash', 'email': 'jonathan.north.washington@gmail.com', 'username': 'jonorthwash'}, 'url': 'https://github.com/apertium/apertium-sah/commit/e7a9ae0550d099fe8b801ca0dcc4f2e9b9620654', 'added': ['tests/run-twol-pairtests.sh'], 'author': {'name': 'jonorthwash', 'email': 'jonathan.north.washington@gmail.com', 'username': 'jonorthwash'}, 'modified': [], 'tree_id': 'addb9fd23180bcf95cefed25f8c24236316635b9', 'removed': [], 'id': 'e7a9ae0550d099fe8b801ca0dcc4f2e9b9620654', 'timestamp': '2018-07-22T22:47:30-04:00', 'message': 'twol pairtest script', 'distinct': True}], 'base_ref': None, 'sender': {'type': 'User', 'events_url': 'https://api.github.com/users/jonorthwash/events{/privacy}', 'url': 'https://api.github.com/users/jonorthwash', 'following_url': 'https://api.github.com/users/jonorthwash/following{/other_user}', 'subscriptions_url': 'https://api.github.com/users/jonorthwash/subscriptions', 'followers_url': 'https://api.github.com/users/jonorthwash/followers', 'login': 'jonorthwash', 'repos_url': 'https://api.github.com/users/jonorthwash/repos', 'received_events_url': 'https://api.github.com/users/jonorthwash/received_events', 'gravatar_id': '', 'organizations_url': 'https://api.github.com/users/jonorthwash/orgs', 'html_url': 'https://github.com/jonorthwash', 'gists_url': 'https://api.github.com/users/jonorthwash/gists{/gist_id}', 'avatar_url': 'https://avatars0.githubusercontent.com/u/963849?v=4', 'id': 963849, 'site_admin': False, 'starred_url': 'https://api.github.com/users/jonorthwash/starred{/owner}{/repo}', 'node_id': 'MDQ6VXNlcjk2Mzg0OQ=='}, 'organization': {'id': 4594256, 'hooks_url': 'https://api.github.com/orgs/apertium/hooks', 'events_url': 'https://api.github.com/orgs/apertium/events', 'members_url': 'https://api.github.com/orgs/apertium/members{/member}', 'repos_url': 'https://api.github.com/orgs/apertium/repos', 'description': 'Free/open-source platform for developing rule-based machine translation systems and language technology', 'url': 'https://api.github.com/orgs/apertium', 'issues_url': 'https://api.github.com/orgs/apertium/issues', 'avatar_url': 'https://avatars2.githubusercontent.com/u/4594256?v=4', 'public_members_url': 'https://api.github.com/orgs/apertium/public_members{/member}', 'login': 'apertium', 'node_id': 'MDEyOk9yZ2FuaXphdGlvbjQ1OTQyNTY='}, 'repository': {'has_projects': True, 'updated_at': '2018-07-21T17:41:45Z', 'homepage': None, 'svn_url': 'https://github.com/apertium/apertium-sah', 'archived': False, 'created_at': 1520473315, 'downloads_url': 'https://api.github.com/repos/apertium/apertium-sah/downloads', 'open_issues_count': 7, 'stargazers_url': 'https://api.github.com/repos/apertium/apertium-sah/stargazers', 'deployments_url': 'https://api.github.com/repos/apertium/apertium-sah/deployments', 'fork': False, 'trees_url': 'https://api.github.com/repos/apertium/apertium-sah/git/trees{/sha}', 'pushed_at': 1532314058, 'issues_url': 'https://api.github.com/repos/apertium/apertium-sah/issues{/number}', 'compare_url': 'https://api.github.com/repos/apertium/apertium-sah/compare/{base}...{head}', 'tags_url': 'https://api.github.com/repos/apertium/apertium-sah/tags', 'master_branch': 'master', 'subscribers_url': 'https://api.github.com/repos/apertium/apertium-sah/subscribers', 'subscription_url': 'https://api.github.com/repos/apertium/apertium-sah/subscription', 'size': 2143, 'git_url': 'git://github.com/apertium/apertium-sah.git', 'name': 'apertium-sah', 'git_commits_url': 'https://api.github.com/repos/apertium/apertium-sah/git/commits{/sha}', 'commits_url': 'https://api.github.com/repos/apertium/apertium-sah/commits{/sha}', 'contributors_url': 'https://api.github.com/repos/apertium/apertium-sah/contributors', 'watchers': 0, 'collaborators_url': 'https://api.github.com/repos/apertium/apertium-sah/collaborators{/collaborator}', 'default_branch': 'master', 'license': {'name': 'GNU General Public License v3.0', 'url': 'https://api.github.com/licenses/gpl-3.0', 'key': 'gpl-3.0', 'spdx_id': 'GPL-3.0', 'node_id': 'MDc6TGljZW5zZTk='}, 'git_tags_url': 'https://api.github.com/repos/apertium/apertium-sah/git/tags{/sha}', 'html_url': 'https://github.com/apertium/apertium-sah', 'private': False, 'archive_url': 'https://api.github.com/repos/apertium/apertium-sah/{archive_format}{/ref}', 'full_name': 'apertium/apertium-sah', 'watchers_count': 0, 'node_id': 'MDEwOlJlcG9zaXRvcnkxMjQzMTk3MjY=', 'teams_url': 'https://api.github.com/repos/apertium/apertium-sah/teams', 'clone_url': 'https://github.com/apertium/apertium-sah.git', 'forks_url': 'https://api.github.com/repos/apertium/apertium-sah/forks', 'issue_comment_url': 'https://api.github.com/repos/apertium/apertium-sah/issues/comments{/number}', 'events_url': 'https://api.github.com/repos/apertium/apertium-sah/events', 'mirror_url': None, 'stargazers': 0, 'releases_url': 'https://api.github.com/repos/apertium/apertium-sah/releases{/id}', 'has_downloads': True, 'language': 'Makefile', 'issue_events_url': 'https://api.github.com/repos/apertium/apertium-sah/issues/events{/number}', 'notifications_url': 'https://api.github.com/repos/apertium/apertium-sah/notifications{?since,all,participating}', 'url': 'https://github.com/apertium/apertium-sah', 'stargazers_count': 0, 'forks': 0, 'owner': {'type': 'Organization', 'events_url': 'https://api.github.com/users/apertium/events{/privacy}', 'url': 'https://api.github.com/users/apertium', 'name': 'apertium', 'gists_url': 'https://api.github.com/users/apertium/gists{/gist_id}', 'subscriptions_url': 'https://api.github.com/users/apertium/subscriptions', 'followers_url': 'https://api.github.com/users/apertium/followers', 'starred_url': 'https://api.github.com/users/apertium/starred{/owner}{/repo}', 'repos_url': 'https://api.github.com/users/apertium/repos', 'received_events_url': 'https://api.github.com/users/apertium/received_events', 'gravatar_id': '', 'organizations_url': 'https://api.github.com/users/apertium/orgs', 'email': 'apertium-contact@lists.sourceforge.net', 'html_url': 'https://github.com/apertium', 'following_url': 'https://api.github.com/users/apertium/following{/other_user}', 'avatar_url': 'https://avatars2.githubusercontent.com/u/4594256?v=4', 'id': 4594256, 'site_admin': False, 'login': 'apertium', 'node_id': 'MDEyOk9yZ2FuaXphdGlvbjQ1OTQyNTY='}, 'merges_url': 'https://api.github.com/repos/apertium/apertium-sah/merges', 'has_wiki': True, 'assignees_url': 'https://api.github.com/repos/apertium/apertium-sah/assignees{/user}', 'blobs_url': 'https://api.github.com/repos/apertium/apertium-sah/git/blobs{/sha}', 'organization': 'apertium', 'keys_url': 'https://api.github.com/repos/apertium/apertium-sah/keys{/key_id}', 'hooks_url': 'https://api.github.com/repos/apertium/apertium-sah/hooks', 'has_issues': True, 'ssh_url': 'git@github.com:apertium/apertium-sah.git', 'languages_url': 'https://api.github.com/repos/apertium/apertium-sah/languages', 'pulls_url': 'https://api.github.com/repos/apertium/apertium-sah/pulls{/number}', 'comments_url': 'https://api.github.com/repos/apertium/apertium-sah/comments{/number}', 'has_pages': False, 'git_refs_url': 'https://api.github.com/repos/apertium/apertium-sah/git/refs{/sha}', 'description': 'Apertium linguistic data for Sakha', 'statuses_url': 'https://api.github.com/repos/apertium/apertium-sah/statuses/{sha}', 'forks_count': 0, 'branches_url': 'https://api.github.com/repos/apertium/apertium-sah/branches{/branch}', 'open_issues': 7, 'contents_url': 'https://api.github.com/repos/apertium/apertium-sah/contents/{+path}', 'id': 124319726, 'labels_url': 'https://api.github.com/repos/apertium/apertium-sah/labels{/name}', 'milestones_url': 'https://api.github.com/repos/apertium/apertium-sah/milestones{/number}'}, 'ref': 'refs/heads/master', 'deleted': False, 'head_commit': {'committer': {'name': 'jonorthwash', 'email': 'jonathan.north.washington@gmail.com', 'username': 'jonorthwash'}, 'url': 'https://github.com/apertium/apertium-sah/commit/e7a9ae0550d099fe8b801ca0dcc4f2e9b9620654', 'added': ['tests/run-twol-pairtests.sh'], 'author': {'name': 'jonorthwash', 'email': 'jonathan.north.washington@gmail.com', 'username': 'jonorthwash'}, 'modified': [], 'tree_id': 'addb9fd23180bcf95cefed25f8c24236316635b9', 'removed': [], 'id': 'e7a9ae0550d099fe8b801ca0dcc4f2e9b9620654', 'timestamp': '2018-07-22T22:47:30-04:00', 'message': 'twol pairtest script', 'distinct': True}, 'compare': 'https://github.com/apertium/apertium-sah/compare/413214eb2738...e7a9ae0550d0', 'created': False, 'pusher': {'name': 'jonorthwash', 'email': 'jonathan.n.washington@gmail.com'}}
ERROR: 'sha'
Androbin commented 6 years ago

The docs list sha but the payload has id anyway, so I've patched that now. You're using the .commit command from the commit module. Please use the new !commit command from the git module.

jonorthwash commented 6 years ago

Is there a way to standardise all this to just .commit? Having !commit too breaks consistency, and is confusing. Perhaps we could do the regexes such that the svn one is activated if the commit it's just numbers, and the git one is activated if it's hex (though there are only-number examples there). Otherwise, maybe rename the svn one to .rev/.revision ?

Androbin commented 6 years ago

I don't know about a command for SVN. .commit from commit.py returns a "What the Commit" commit message. Not even sure if we want to keep that module.