sopel-irc/sopel-github

GitHub plugin for Sopel

sopel-github's Issues

Fix setting names

After spending literally hours chasing our tails over a "bug" that turned out to be a perfectly sensible (but incorrect) setting name in this module's section, we came to one conclusion:

github.secret should definitely be named github.client_secret, to match github.client_id and the GitHub application settings UI.

To be implemented no earlier than 0.2.0—this is a breaking change that will require users to edit their Sopel configuration files, though a migration is possible to build in. Perhaps switch to use the new setting name if it exists and fall back to the old one for a few releases before dropping support for it entirely some months later.

git.io is closed

GitHub shut down new links at git.io this week with no advance warning. This plugin has been using the service to shorten the URLs in webhook events:

sopel-github/sopel_modules/github/formatting.py

Line 573 in 5fe16e4

res = requests.post('https://git.io', {'url': url})

We need an alternative, and I'm sad to say the only viable options might be TinyURL or Bitly. Those are the only URL shorteners I can think of that have lasted longer than git.io's 10 years.

Fortunately nothing is broken, since this plugin also gracefully handles failure to shorten links. Sending the POST request is now a waste of time (because it is guaranteed to be rejected with an error), but webhooks and such will still be sent to IRC with the original URL.

Starring can be abused

Repeatedly starring/unstarring projects monitored by sopel-github will cause spam.

Inline issue references generate error spam

In a channel where people like to talk about numbered stuff in contexts other than issues, it's pretty easy to trigger errors from sopel-github when a number higher than the maximum issue/PR number in the linked repo is mentioned.

We should suppress "[GitHub] API says this is an invalid issue" errors for inline refs without a (user/)repo attached.

PR merge messages need more info

Current message when a PR is merged:
<Sopel> [sopel] dgw approved pull request #1516 https://git.io/fjqGw
This should have at least the name of the user who opened it and the title, like normal PR link info display:
<Sopel> [GitHub] [sopel-irc/sopel #1539] deathbybandaid: Provide a simpler way to kick users from channels. | this adds a `bot.kick(text, channel, nick)` ability to the bot, instead of using `bot.write(['KICK', channel, nick], text)`. ...

It would be good to check other simple action messages, like issue/pr closed, for format consistency or similar missing info.

"Last Push" for a repo can be misleading

Anecdotally, the best way to see if a repo is still "active" is to look at how long it's been since the last commit to the repo's default branch. In most cases, how old that commit is is a decent proxy for how much development is happening.

GitHub's API doesn't return that value for the repo object's updated_at or pushed_at fields, though. pushed_at includes any ref/HEAD, including PR branches that might originate in a fork (because of GitHub internal implementation stuff). updated_at can be changed by any number of events, including issue/PR comments from random users, and who knows what else.

It is possible to take a repo's default_branch value and make a second API call to https://api.github.com/repos/{owner}/{repoName}/branches/{branchName}, then fetch commit.commit.author.date or commit.commit.committer.date* from there—but that is a lot of work for perhaps not much gain.

For that reason I'm not saying that this issue should be implemented. I'm just opening it so we remember this shortcoming of how things work now. At some point the plugin should be rewritten to use GitHub's GraphQL API, and that should let us fetch exactly the fields we want for any given handler without making multiple calls.**

---

* — This comes back as nested dictionaries in JSON, of course, but dot notation is easier to read than consecutive subscripts.

** — One downside: GitHub's GraphQL endpoint allows no unauthenticated access, unlike the REST API.

Option to ignore short unprefixed inline issue numbers

It really seems like most projects that seriously use a plugin like this are going to have at least double-digit issues as the active set. Parsing the message <thing> is my #1 favorite into an issue is never useful in Sopel's own channel.

Related to #127 but not the same (that's for a heuristic; this is for a configuration setting/s).

It's possible that this would be better as a per-channel value like the "linked repo". A global setting is simpler, though.

Improvements that can be made for Sopel 7.1

sopel-github/sopel_modules/github/github.py

Lines 122 to 128 in 273fed4

	@rule(r'.*(?<!\S)/?#(\d+)\b.*')
	@require_chanmsg
	def issue_reference(bot, trigger):
	"""
	Separate function to work around Sopel not loading rules/commands for @url callables.
	"""
	issue_info(bot, trigger)

This can use the new-to-7.1 sopel.plugin.find decorator, which "Decorate[s] a function to be called for each time a pattern is found in a line." Doing so will fix the current behavior, which is to show information only about the last issue/PR reference in a given line.

Can also combine it with the @url-decorated callable instead of having a separate one, since Sopel 7.1 rewrote a bunch of logic and added tests to make sure that the triggerable decorator types should all work together. Not yet, actually. See sopel-irc/sopel#2378.

---

I will update this comment as I think of more things that can be improved or simplified.

IRC message output should expose abbreviated commit checksum

Current output:

15:59:36 <Sopel> [sopel] lgtm-com[bot] commented on pull request #1904: This pull request **introduces 1 alert** when
                 merging 3c3518032f0e9ca858e597d5c0691ea3a229b2db into ee3fcb2ab8cfea3e08f8cc9b3997f2ba32fa5494
                 - [view on LGTM.com](https://lgtm.com/projects/g/sopel- […] https://git.io/JJKrs

Desired output (or something similar):

15:59:36 <Sopel> [sopel] lgtm-com[bot] commented on pull request #1904: This pull request **introduces 1 alert** when
                 merging 3c35180… into ee3fcb2… - [view on LGTM.com](https://lgtm.com/projects/g/sopel- […]
                 https://git.io/JJKrs

Realizing it's not as simple as it sounds perhaps, but would be nice.

Edit: This would also free up some not-insignificant amount of characters for additional, more useful, output, while staying under the RFC-spec 512 bytes per line limit.

Trailing punctuation on GitHub URLs isn't ignored

If someone posts a GitHub URL that should be recognized and processed, e.g. https://github.com/sopel-irc/sopel-github/issues/7, but it is followed by punctuation (like ,), the plugin will not process it.

URL parsing, and deciding when punctuation should be included as part of a URL especially, is a giant can of worms. This isn't the end of the world (users will learn not to put punctuation right at the end of a URL), but ideally it wouldn't be an issue. Notably, my IRC client (The Lounge) recognizes that , isn't part of the URL in this case.

Can't load on new Sopel setup

Not sure if I'm doing something wrong, but I'm running sopel in docker. I added this package to the list to automatically download, which it does.

Then in a buffer with sopel I run .load github and I am presented with

SyntaxError: invalid syntax (file "/home/sopel/.local/bin/python3.6/site-packages/sopel_modules/github/webhook.py, line 167)

I'm not doing anything with webhooks, would just like to query the occasional repo...

use_aliases is no longer a recognized option in emoji.emojize()

sopel-github/sopel_modules/github/formatting.py

Line 31 in 588db54

emojize = lambda text: emoji.emojize(text, use_aliases=True)

Please remove the option use_aliases=True from this line. If you prefer, I can also send you a PR - it's pretty straight forward.

`git.io` links

Would be nice to handle pasted git.io URLs in chat, too, if they point to resources the plugin knows how to display.

I guess this would mean a HEAD call, plus "faking" the URL handling somehow (but only for this plugin's URL handlers).

[Feature] Listen for standalone ticket references

A number of IRC bots that support GitHub also detect issue/PR references without requiring that the reference be a URL. For example, if someone says, Yeah, that happens because of a regression in #9001, the bot will dig up the details for #9001 and send them to the channel.

sopel-github doesn't currently do this, but it would probably be a useful function for Sopel's own IRC channel, since issue numbers get thrown around there a fair bit. An admin-/op-only command like .gh-hook would need to be added for configuring the target repo for any given channel.

Conveniently, GitHub's issues API endpoint returns data for PRs too, so there's no need to check which type an in-text reference to #9001 is before fetching its info. (The id returned for a PR differs depending on which endpoint is used, but we don't use that particular property anywhere.)

Plugin should handle API timeouts

Just a quick note that occasionally, sending the API request to get information about a repo/issue/PR/etc. will fail and eventually yield an error on IRC such as:

<+Sopel> Unexpected ConnectTimeout (HTTPSConnectionPool(host='api.github.com', port=443): Max retries exceeded with url:
         /repos/znc/znc/issues/1224 (Caused by ConnectTimeoutError(<urllib3.connection.HTTPSConnection object at
         0x7f5d8fac0070>, 'Connection to api.github.com timed out. (connect timeout=None)'))) from dgw. Message was:
         https://github.com/znc/znc/issues/1224

The full traceback is quite messy because it's nested exceptions:

Traceback mess in all its glory

[2024-02-24 02:57:11,388] sopel.bot            ERROR    - Unexpected ConnectTimeout (HTTPSConnectionPool(host='api.github.com', port=443): Max retries exceeded with url: /repos/znc/znc/issues/1224 (Caused by ConnectTimeoutError(<urllib3.connection.HTTPSConnection object at 0x7f5d8fac0070>, 'Connection to api.github.com timed out. (connect timeout=None)'))) from dgw. Message was: https://github.com/znc/znc/issues/1224
Traceback (most recent call last):
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/urllib3/connection.py", line 174, in _new_conn
    conn = connection.create_connection(
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/urllib3/util/connection.py", line 95, in create_connection
    raise err
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/urllib3/util/connection.py", line 85, in create_connection
    sock.connect(sa)
TimeoutError: [Errno 110] Connection timed out

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/urllib3/connectionpool.py", line 703, in urlopen
    httplib_response = self._make_request(
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/urllib3/connectionpool.py", line 386, in _make_request
    self._validate_conn(conn)
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/urllib3/connectionpool.py", line 1042, in _validate_conn
    conn.connect()
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/urllib3/connection.py", line 358, in connect
    self.sock = conn = self._new_conn()
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/urllib3/connection.py", line 179, in _new_conn
    raise ConnectTimeoutError(
urllib3.exceptions.ConnectTimeoutError: (<urllib3.connection.HTTPSConnection object at 0x7f5d8fac0070>, 'Connection to api.github.com timed out. (connect timeout=None)')

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/requests/adapters.py", line 489, in send
    resp = conn.urlopen(
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/urllib3/connectionpool.py", line 787, in urlopen
    retries = retries.increment(
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/urllib3/util/retry.py", line 592, in increment
    raise MaxRetryError(_pool, url, error or ResponseError(cause))
urllib3.exceptions.MaxRetryError: HTTPSConnectionPool(host='api.github.com', port=443): Max retries exceeded with url: /repos/znc/znc/issues/1224 (Caused by ConnectTimeoutError(<urllib3.connection.HTTPSConnection object at 0x7f5d8fac0070>, 'Connection to api.github.com timed out. (connect timeout=None)'))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/sopel/sopel-git/sopel/bot.py", line 695, in call_rule
    rule.execute(sopel, trigger)
  File "/home/sopel/sopel-git/sopel/plugins/rules.py", line 1274, in execute
    exit_code = self._handler(bot, trigger)
  File "/home/sopel/sopel-git/sopel/plugins/rules.py", line 1792, in handler_match_wrapper
    return handler(bot, trigger, match=trigger)
  File "/home/sopel/sopel-github/sopel_modules/github/github.py", line 174, in issue_info
    raw = fetch_api_endpoint(bot, URL)
  File "/home/sopel/sopel-github/sopel_modules/github/github.py", line 125, in fetch_api_endpoint
    return requests.get(url, headers={'X-GitHub-Api-Version': '2022-11-28'}, auth=auth).text
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/requests/api.py", line 73, in get
    return request("get", url, params=params, **kwargs)
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/requests/api.py", line 59, in request
    return session.request(method=method, url=url, **kwargs)
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/requests/sessions.py", line 587, in request
    resp = self.send(prep, **send_kwargs)
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/requests/sessions.py", line 701, in send
    r = adapter.send(request, **kwargs)
  File "/home/sopel/.local/lib/pyenv/versions/3.10.7-sopel-libera/lib/python3.10/site-packages/requests/adapters.py", line 553, in send
    raise ConnectTimeout(e, request=request)
requests.exceptions.ConnectTimeout: HTTPSConnectionPool(host='api.github.com', port=443): Max retries exceeded with url: /repos/znc/znc/issues/1224 (Caused by ConnectTimeoutError(<urllib3.connection.HTTPSConnection object at 0x7f5d8fac0070>, 'Connection to api.github.com timed out. (connect timeout=None)'))

Webhook server can block

We had the webhook server start yielding timeouts when GitHub sent payloads to it.

Unfortunately there was nothing useful in the bot's logs. However, we know that wsgiref is not particularly suited for production use, and there's probably a better ServerAdapter that we could use with Bottle to get things the built-in WSGI reference implementation doesn't offer (higher concurrency, control over and protection from bad requests that never terminate, etc.).

Links with fragments fall through to Sopel's basic URL handler

Tin. Posted https://github.com/pyenv/pyenv#uninstalling-pyenv in chat today and got output from the built-in url plugin.

Undecided whether this should print the repo info or do something else, but letting the built-in handle it seems wrong. Especially because the title on GH is pretty useless (just says "GitHub - user/repo: short description").

Notification spam for related actions

In an ideal world, this plugin would output only one message for situations like these examples:

• new issue/PR opened with labels/milestone/assignee (others?) already set
  • target output: a single unified presentation of all details pertaining to the new issue/PR, on one line (though it may use max_messages to have Sopel split a long line)
• milestone removed, then another added
  • target output: a single "updated the milestone" message
• multiple labels added/removed in quick succession
  • target output: a single message listing the labels added and removed

I think these fairly common occurrences can be built with future expandability in mind. Coalescing notifications like this should happen only for events on the same issue/PR from the same user.

Additional issue/PR event link handling

Re: IRC discussion
The utility of issue display can be improved by parsing the "hash" part of URLs and displaying the linked comment instead of only the original issue/PR description.

Current:

<+dgw> Edited to add: "Meh." :P https://github.com/sopel-irc/sopel-github/pull/26#pullrequestreview-226279842                    
< Sopel> [GitHub] [sopel-irc/sopel-github #26] HumorBaby: webhook: restrict incoming requests | A bad actor can use this [snip multi-line description]

An improvement would be something like:

< Sopel> [GitHub] [sopel-irc/sopel-github #26] HumorBaby: webhook: restrict incoming requests | dgw comments: Let's call it ready. Maybe this will go into 0.2.0 after all! haha

Doesn't ignore multi-line <!-- HTML comments --> in comment bodies

I'm afraid this will require rewriting the simple logic for removing headings, comments, etc. to use a parser of some kind… but the current behavior is annoying nonetheless.

Diffstat for PRs

Maybe useful to include diffstat (+/- line count, # of files changed) in PR link output?

Webhooks include this info when a PR is opened ("additions", "deletions", and "files_changed" keys are directly inside the "pull_request"). Commit count is obviously also available, but probably less useful than the space it would consume in the output IRC line (debatable, of course).

Showing it for linked PRs would be a bit more annoying, as it requires specifically calling the pulls API endpoint instead of the issues one to get the relevant keys. Would have to complicate the logic/matching for issue_info() somewhat.

Channel-value method arguments are in the wrong order

Back in #65, implementation of the .gh-repo command swapped the channel and key arguments to set_channel_value and friends. Since this plugin consistently uses the incorrect order, this hasn't been a problem; it just looks weird if one examines the DB directly.

Still, this should probably get fixed with some form of automatic migration. We can set a flag using plugin_values to indicate whether the migration has been run, too, now that such a feature exists in core.

Possible `current_payload` race condition

Once upon a time, I saw a new PR opened fail to message the channel due to a KeyError: 'pull_request', implying that current_payload may have been replaced by a newer one during processing.

I forgot about it, but fortunately it did get written down somewhere so I could rediscover its existence and open an issue.

All of that current_payload nonsense really has to go. Every time I look at this plugin I find more things that desperately need refactoring…

Support arbitrary inline issue references

Inline issue references for the channel's configured repo are great, but what about supporting the owner-name/repo-name#number format so people can mention issues that aren't in the current channel's linked repo?

More messages need issue/PR titles

It's not super useful to see [sopel] dgw added the label 'Declined' to issue #343 without knowing what issue 343 is. Labels aren't the only ones, either; milestones, too, and probably others I can't think of just now (am multitasking on a conference call like a bad boy).

Add link to issue/PR output when triggered by numeric reference

That is, at present there's no way to get to the GitHub page if someone brings up an issue or PR by mentioning its number in a channel where that option is enabled.

<cottongin> #2014
<Sopel> [GitHub] [sopel-irc/sopel #2014] cottongin: find: format replacement text instead
        of "meant" | This simply applies the bold formatting to whatever the new replacement
        text is rather than the "meant" text. […]

Should have been added along with #65 but oh well.

Error: "Address already in use" when Sopel reconnects with webhooks enabled

We've been seeing OSError: [Errno 98] Address already in use on the "official" Sopel instance when something interrupts its ZNC connection. (Later, failure of the webhook server to bind at setup() during reconnection leads to AttributeError: 'NoneType' object has no attribute 'shutdown' the next time Sopel exits or reconnects, since this plugin's shutdown() routine doesn't correctly check if the server object is valid before trying to shut it down.)

Sopel reconnecting is very similar to what happens during stop-start or restart: It shutdown()s all plugins, waits a bit, then setup()s all the plugins again. The difference is, it's the same process running those setup()s again instead of a new one. But interestingly, it's not possible to trigger the same behavior by starting sopel, doing Ctrl-C, and immediately starting sopel again.

It seems like the listener socket (from Bottle) is getting stuck in TIME_WAIT for a few minutes because the local end initiated the closure (this is just part of how TCP works). When the whole process terminates, the leftover socket is probably freed by the OS—but not when the process stays running, i.e. during a reconnection.

I'm not quite done researching the flag SO_REUSEADDR and its possible (side-)effects or other considerations, but it might help. Probably would require a custom Bottle backend to set up the option.

shorten_url() truncates GET params

Helped @bezaban with this module on IRC earlier and traced the issue to shorten_url() cutting GET parameters off of the OAuth request URL when it got shortened. I should get around to making the fix official soon enough, but in case I don't this issue is here to act as a reminder. (As discussed on IRC, I'm in the "test first, commit later" camp, not the "commit first, test later" one. Though I'm testing the fix in production, so…)

Anyway, requests takes a string passed as data literally and doesn't encode it at all. Long story short, that means the URL to be shortened by git.io is truncated at the first & as currently implemented, which breaks OAuth requests. The solution should be pretty simple—pass url as a dict key instead of a string. And if no side effects show up in my testing, that's what I'll do. (I don't expect to see any side effects. Honestly, the way shorten_url() is written now probably never worked properly.)

Need a way to choose enabled events locally

By default, this plugin enables all events, which can be spammy for active projects. Even projects with minimal activity might not want all supported events sent to IRC.

For example, we've configured Sopel's own webhook to send mostly issue- or PR-related events, and ignore stars/pushes.

Right now, the only way to disable certain events is to edit the webhook settings on GitHub. Eventually, we'd like to support filtering events on the bot end, to simplify the setup process (no need to open up GitHub after enabling the webhook). The biggest challenge will be designing the commands ("UI") to manage this, especially since it'll be done on a per-pair (repo:channel) basis—it's not likely to be something we can easily put in the config wizard.

Implementing this ourselves would also permit more granular filtering than GitHub supports, e.g. receiving issue open/close events but not labels/milestones/edits.

Bad plural in commit output ("1 changes in 1 file")

We need #37 but for the other part of that message.

Why didn't I fix both at once 3 years ago? Not a clue, lol.

OAuth client_id and client_secret method deprecated

Receiving notifications from GitHub that authentication method of using client_id and client_secret is deprecated. Using personal tokens via Basic Authentication is the recommend route to authentication.

From Blog Post

Authenticating using query parameters
GitHub is deprecating authentication to the GitHub API using query parameters, such as using a access_token query parameter for OAuth user authentication or a client_id/client_secret query parameter for OAuth application authentication. All authentication to the GitHub API should be done using HTTP basic authentication.

Broken on Sopel master (7.0.0-dev)

After updating my own Sopel bot to master, the GitHub plugin stopped loading. It seems related to the webhook functionality:

[2019-10-31 18:45:50,344] sopel.modules.reload ERROR    - Error loading github: 'Connection' object has no attribute 'cursor'
Traceback (most recent call last):
  File "/usr/local/lib/python3.5/dist-packages/sopel/modules/reload.py", line 26, in _load
    plugin.setup(bot)
  File "/usr/local/lib/python3.5/dist-packages/sopel/plugins/handlers.py", line 252, in setup
    self._module.setup(bot)
  File "/usr/local/lib/python3.5/dist-packages/sopel_modules/github/github.py", line 96, in setup
    setup_webhook(sopel)
  File "/usr/local/lib/python3.5/dist-packages/sopel_modules/github/webhook.py", line 50, in setup_webhook
    c = conn.cursor()
AttributeError: 'Connection' object has no attribute 'cursor'

Since the custom DB stuff in this plugin was written for when Sopel used SQLite directly, and it now uses SQLAlchemy to abstract the database away, I'd say that's what broke this.

I'm doubtful that we can fix this on the Sopel side, so I'm opening the issue here. SQLAlchemy just won't be the same as using sqlite3.connect() because, well, it's different. @RustyBower your input/help patching would be much appreciated, especially if you can tell me that this is fixable on the Sopel side so we can keep old code working! 😉

(Please don't judge me for py3.5; it's the newest I could install without compiling my own. 😅)

Issue/PR info should include closed/open/merged status

Sopel showing the open/closed/merged status of issues and PRs would save me a click sometimes. Ideally issues could show open/closed/PR/merged or something so I can assume more, but that would probably be a another API call.

Current:

< Sopel> [GitHub] [sopel-irc/sopel #1810] deathbybandaid: UnrealIRCd/InspIRCd Channel History repetition |
    I got around to uprading my UnrealIRCd to version 5, which supports more IRCv3 features. One of which,
    I was really excited about for my friends that connect is channel history. […] | https://git.io/Jvt3i

Maybe < Sopel> [GitHub] [sopel-irc/sopel #1810] [open] deathbybandaid: UnrealIRCd...?

Currently no output for Discussion links

Discussions (e.g. Sopel's) are a fairly new thing, still marked as Beta in many places, but it would be nice to handle links to them and at least show basic info:

• Title
• Number
• Starter/author
• Category
• Body snippet (?)

Will track webhook support separately, as that will have its own bikeshedding to do.

Only the last of multiple GitHub URLs is processed (sometimes)

If someone sends multiple issue URLs in one line:

<dgw> https://github.com/ircv3/ircv3-ideas/issues/33
      https://github.com/ircv3/ircv3-ideas/issues/32

only the last one is processed:

<Sopel> [GitHub] [ircv3/ircv3-ideas #32] jesopo: Content warnings | I'd like to propose
        using a `message-tag` on a `PRIVMSG` to denote a message with a content warning. …

The rest are simply ignored.

Things get more interesting when both issue/PR and commit URLs are sent in the same line: The last match for each will be processed.

Each @rule decorator can match only once, so it makes sense that the module works this way.

This issue is probably specific to Sopel instances with the core url module disabled, like mine. All the url_callbacks stuff appears to be registered properly for when url is loaded.

It's probably not worth fixing this separately (with tweaks to the regex patterns or whatever). Until sopel-irc/sopel#1489 is fixed, the easiest and best solution—using @url instead of @rule—would break this module's URL-processing on instances that do not load the core url module.

We should probably just note that the issue exists, and fix it once and for all when the @url decorator is fixed to work properly in all cases.

No 'state' included with issue/PR comments

Looks like #87 broke fetching comments because there won't be a state key in the returned JSON.

If it's important to know whether the issue/PR commented upon is open or closed, this will always require two API calls to display comments. If not important, that part of the logic can just be skipped for comment display.

Simplistic filtering of headings in comment body

This code is a bit too simplistic:

sopel-github/sopel_modules/github/formatting.py

Line 83 in bb66e62

and not line.startswith('#') # Markdown heading

It doesn't allow for one-line comments that start with an issue/PR/discussion reference, like this one I just left in Sopel's main repo.

Commit info output should include date

Tin. I'm sure the GitHub API request the plugin already sends to get the commit info already includes this.

Decisions will be:

• Committer date or author date? (both?)
• Absolute (sopel.tools.time.format_time()) or relative time (sopel.tools.time.seconds_to_human())?

Code line preview doesn't work with GET parameters

GitHub puts the view mode into the URL sometimes when viewing renderable files (Markdown, reST, etc.), which can break sopel-github parsing the link:

<dgw> https://github.com/Exirel/sopel/blob/34777e2500f713b250413b6d3c978004e04497c3/docs/source/tutorials/configurable.rst?plain=1#L65

(That is, Sopel doesn't recognize that it should handle the link.) Compare to the plugin behavior without ?plain=1:

<dgw> https://github.com/Exirel/sopel/blob/34777e2500f713b250413b6d3c978004e04497c3/docs/source/tutorials/configurable.rst#L65
<Sopel> [GitHub] [Exirel/sopel] docs/source/tutorials/configurable.rst @ 34777e2500f713b250413b6d3c978004e04497c3 | L65: bot.say(f'I want a {fruit}!')

Error on missing commit author login

As an example, sopel-irc/sopel@30cddb2 yields the following error in the github plugin:

TypeError: 'NoneType' object is not subscriptable (file
"/usr/local/lib/python3.6/site-packages/sopel_modules/github/github.py",
line 209, in commit_info)

Assuming that instance is up to date, that line number points to

sopel-github/sopel_modules/github/github.py

Line 209 in 08a816a

data['author']['login'],

It makes sense, since that commit appears not to be associated with a GitHub user. I'd need to inspect the API response to be sure, but I'm reasonably confident that the issue is there.

Configurable notification limits

The code is very naïve right now, and just spits out a list of commits pushed. Even if there are dozens of them. That's not very friendly unless the module is enabled in a dedicated "dev" channel (and even then, it runs into flood protection pretty quickly). We should have a way to set limits on events like pushes, which can possibly contain many sub-items.

I would have just knocked this out real quick, but there's no way to access bot.config from the code that generates the messages. So it's a bigger project, suitable for the next version bump instead of the next patch release, and some thought will have to go into how passing the options around should work (so other hooks' behaviors can also be made configurable using the same tools).

The best way might be storing an options object (JSON or whatever, the format isn't important) in the database alongside the repo:channel pairs, since the plugin has to go fetch things from there anyway (like the colors to use).

Error loading github: cannot import name formatting (../../sopel_modules/github/github.py:21)

Trying to run this on OSX 10.10.5, I did install via pip install sopel_modules.github.

Any idea?

Regex Failure

Current regex doesn't match https://github.com/gnomon-/furby-source

GH topic links handled improperly

22:11:37 < xnaas> ... https://github.com/topics/sopel-plugin ...
22:11:37 <+Sopel> [GitHub] Not Found

I suspect it's parsing the topic link as a project link, user=topics repo=sopel-plugin

Function post is deprecated.

Hoping this project is still alive...

Function post is deprecated.
  File "/usr/lib/python2.7/threading.py", line 783, in __bootstrap
    self.__bootstrap_inner()
  File "/usr/lib/python2.7/threading.py", line 810, in __bootstrap_inner
    self.run()
  File "/usr/lib/python2.7/threading.py", line 763, in run
    self.__target(*self.__args, **self.__kwargs)
  File "/usr/local/lib/python2.7/dist-packages/sopel/bot.py", line 470, in call
    exit_code = func(sopel, trigger)
  File "/usr/local/lib/python2.7/dist-packages/sopel/module.py", line 304, in _nop
    return function(*args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/sopel_modules/github/github.py", line 370, in configure_repo_messages
    bot.say('Great! Please allow me to create my webhook by authorizing via this link: ' + shorten_url(auth_url))
  File "/usr/local/lib/python2.7/dist-packages/sopel_modules/github/formatting.py", line 350, in shorten_url
    res, headers = web.post('http://git.io', 'url=' + web.quote(url), return_headers=True)
127.0.0.1 - - [05/Dec/2017 15:28:48] "GET /webhook?code=blahblahblah&state=repoowner%repo%3A%23%subscriber HTTP/1.0" 200 34

Auto-adding repo webhook is broken

I think we need a pretty full-on rewrite of how the authorization flow works anyway (like everything else related to the webhook handling), but let's be sure to note that the current implementation appears to be broken.

I executed .gh-hook sopel-irc/sopel-github enable in a new channel for us on Libera and clicked the authorization link, but nothing happened. No new webhook appeared in either the repository or organization settings, and Sopel never confirmed that it had received a test payload.

Checking the console log output didn't show anything useful, though. The only things related to this plugin were URL shortening errors (because of #102) and a GET request generated when my browser was redirected after authorizing the request. I didn't see anything about the backend process related to adding the webhook itself.

The reason

Turns out there were multiple issues. First, the OAuth app configuration was wrong (using /webhook instead of /auth as the authorization callback). But more importantly, GitHub has deprecated passing auth tokens via URL, as I found out by modifying the plugin to dump its JSON payload:

Reading the linked developer blog post gives the full background, but it's clear that the webhook creation process here needs to be rewritten to use an Authorization header.

Should probably add brief documentation on manually adding the webhook in the 0.4.x series. I doubt this rewrite is going to happen unless it's part of that big rewrite-all-the-webhook-stuff refactor we've wanted to do for a while.

Feature Request: Differentiate Pulls/Issues in Output

Right now the bot says something like:

[GitHub] helix-editor/helix - A post-modern modal text editor. | 83.6% Rust 15.1% Scheme 0.6% Handlebars 0.7% Other | Last Push: 2023-08-19 @ 05:00CDT | Stargazers: 24179 | Watchers: 163 | Forks: 1740 | Network: 1740 | Open Issues: 1111

Ideally it would say (at this exact moment), something like:

[...] | Open Issues: 806 | Open PRs: 305

or

[...] | Open Issues: 806 | Open Pulls: 305

---

If the GitHub API makes this impossible...well...curses M$FT.

Error when using private repo and webhook: OperationalError: database is locked

<Strykar> .gh-hook user/projecct
<sopel> Successfully enabled the subscription to user/project's events

<sopel> Great! Please allow me to create my webhook by authorizing via this link: https://github.com/login/oauth/authorize?scope=write%3Arepo_hook&state=user%2Fproject%3A%23sopel&client_id=xxxxx

<sopel> Once that webhook is successfully created, I'll post a message in here. Give me about a minute or so to set it up after you authorize. You can configure the colors that I use to display webhooks with .gh-hook-color

<sopel> OperationalError: database is locked (file "/usr/local/lib/python2.7/dist-packages/sopel/db.py", line 64, in execute)

Line 64 in db.py is actually blank.

Add handling of Discussion webhooks

List of webhook actions to handle for Discussions:

created
edited
deleted
pinned
unpinned
locked
unlocked
transferred
category_changed
answered
unanswered

And Discussion comments:

created
edited
deleted

Responses to links should include time information

<+dgw> 💭(github plugin's comment output should include the date...)
<+dgw> or relative timestamp

+1, links to comments (and issues!) would benefit from some additional information about when the linked-to thing occurred