Migrate to Discord 2.0a0 #1815

Akarys42 · 2021-09-06T13:57:54Z

Since the Discord.py repository has been archived, we can switch to the latest commit of 2.0a0, knowing no breaking change will occur (still pinned to the commit just in case).

This PR fixes any problem related to the migration:

New avatar interface
TZ aware datetimes
Various internal API changes

Additionally, this PR adds the following:

Automatically join new threads
Plug threads into mod-log
Respect the parent channel when checking for a potential modlog blacklisting
Forbid threads from being silenced
Remove threads related perms when a channel is silenced ~~or defcon shutdown is used~~

The bot will also automatically join new threads and silence cannot be used in threads due to their lack of permissions.
This was a significantly harder migration than Sir Lance, but I have tested every cog individually, we should hopefully not run into any issue.

Since the Discord.py repository has been archived, we can switch to the latest commit of 2.0a0, knowing no breaking change will occur (still pinned to the commit just in case). This commits fixes any problem related to the migration: - New avatar interface - TZ aware datetimes - Various inernal API changes

Kronifer

Looks good!

Bluenix2

All I could find from the diff, but I'll have to check what you didn't change as well to make sure we do not miss something.

Bluenix2 · 2021-09-17T12:48:17Z

bot/exts/moderation/defcon.py

+            create_private_threads=None,
+            create_public_threads=None,


This won't re-enable them if they were previously enabled, or are we gonna use channel overrides here instead enabling it globally?

Yeah I don't think we're planning on enabling it globally atm

But considering that, I'm not sure these commands should do anything with thread permissions at all?

It should, you can open threads on messages even if you don't have send message permissions.

And if you have the "Send messages in threads" then you can send messages in that thread.

Yes, "Send messages in threads" should be changed, but these two are off by default.

I believe that's two perms we want to enable per channel, not globally

Yeah, so I don't think defcon should mess with these two globally.

bot/exts/moderation/modlog.py

Bluenix2 · 2021-09-18T00:00:31Z

bot/exts/moderation/silence.py

+            count=None,
+            reconnect=True,
+            loop=None,
+            time=MISSING


Huh? Shouldn't MISSING be the keyword argument's default already?

Huh?

That was also my reaction :D but apparently due to how the API changed you have to set it explicitly

bot/exts/utils/bot.py

Co-authored-by: Bluenix <bluenixdev@gmail.com>

bot/exts/moderation/modlog.py

Bluenix2 · 2021-09-19T13:44:07Z

bot/exts/moderation/modlog.py

+        # If we are in the thread already we can most probably assume we already logged it?
+        # We don't really have a better way of doing this since the API doesn't make any difference between the two
+        if thread.me:
+            return


So this is sent when a thread is created, or we get access to a private thread by being mentioned in it.

We don't need this, when this event is received it should be from a thread being created no?

I don't think you are looking at the right event. The API event for on_thread_join seems to be Thread Create, which fires both when a new thread becomes accessible by the bot and when the bot actually joins it.

Without this check, we would log the thread twice, once when it is created and the even first fires, and a second time when it actually joins the thread. That's what I could also check during my testings.

It also fires when anyone joins a thread.

Ahhh, I took a screenshot of the right event at first but then mixed it up.

Yes, Thread Update is the one that is sent when people join. But Thread Create is only sent when a thread is made available to the bot.

yes, which I believe is the one used by on_thread_join, since its behavior matches that. So we do need to have this check.

I don't understand, I am still not convinced. Thread Update is the one that gets sent when a thread gets updated, which it does when members join because now the list of joined members change.

Indeed, but I am not sure how that's relevant? I could confirm through testing that's the behaviour we want

Bluenix2 · 2021-09-19T13:48:47Z

bot/exts/utils/bot.py

+    @Cog.listener()
+    async def on_thread_join(self, thread: Thread) -> None:
+        """
+        Try to join newly created threads.
+
+        Despite the event name being misleading, this is dispatched when new threads are created.
+        """
+        if thread.me:
+            # We have already joined this thread
+            return
+
+        with suppress(Forbidden):
+            await thread.join()


Is this because we want the bot to appear in the right side?

Yep! It first was to make it listen to messages, but we deemed that as unnecessary. I still find it nice to have the bots in the member list.

Co-authored-by: Bluenix <bluenixdev@gmail.com>

Defcon won't touch global thread perms anymore

HassanAbouelela

This PR is not ready to be merged once reviewed, please wait for the great merge event.

Policy is now enforced by review bot.

brad90four

LGTM

ChrisLovering

Looks like we've missed some datetime comparisions here:

https://github.com/python-discord/bot/blob/main/bot/exts/moderation/defcon.py#L114 now is datetime.utcnow() so it naive
https://github.com/python-discord/bot/blob/main/bot/exts/moderation/voice_gate.py#L168

FWIW I think we shouldn't be stripping the tzinfo from timestamps from discord, but instead should be using arrow.utcnow() instead of datetime.utcnow() in those cases since arrow.utcnow() is aware

Akarys42 · 2021-10-15T09:36:01Z

FWIW I think we shouldn't be stripping the tzinfo from timestamps from discord, but instead should be using arrow.utcnow() instead of datetime.utcnow() in those cases since arrow.utcnow() is aware

Would there be a difference between the two?

ChrisLovering · 2021-10-15T09:42:16Z

FWIW I think we shouldn't be stripping the tzinfo from timestamps from discord, but instead should be using arrow.utcnow() instead of datetime.utcnow() in those cases since arrow.utcnow() is aware

Would there be a difference between the two?

No functional difference with just this change, but rather since Discord.py now uses aware, it's probably better we use aware too, rather than changing Discord.py's to naive, this also encourages the use of arrow with our contribs, which has far less pitfalls that datetime, due to legacy constraints in datetime

ChrisLovering

Looks good from my testing, my comment about using arrow.utcnow() rather than stripping tzinfo from D.py's dates isn't required.

Requested changes applied

Akarys42 added 3 commits September 6, 2021 15:35

Make the bot auto-join threads

ab9132a

Silence: forbid threads from being silenced

6b20bfb

Akarys42 added t: feature New feature or request a: dependencies Related to package dependencies and management p: 2 - normal Normal Priority t: enhancement Changes or improvements to existing features labels Sep 6, 2021

Akarys42 requested review from Den4200, jb3, ks129 and mbaruh as code owners September 6, 2021 13:57

Xithrius added the s: needs review Author is waiting for someone to review and approve label Sep 8, 2021

Akarys42 added 2 commits September 12, 2021 15:50

Modlog: respect blacklist with threads

31a7a67

Silence: remove thread related permissions

3c80c3a

Kronifer approved these changes Sep 13, 2021

View reviewed changes

Akarys42 added 3 commits September 15, 2021 10:32

Silence tests: use thread perms

7ce571c

Defcon: add thread perms to (un)shutdown

11147c5

Modlog: thread support

def3a5c

Xithrius requested review from bast0006 and Bluenix2 and removed request for ks129 and Den4200 September 16, 2021 22:13

Bluenix2 reviewed Sep 18, 2021

View reviewed changes

Akarys42 and others added 3 commits September 19, 2021 15:24

Modlog: reuse logic for the message blacklist

5024380

Co-authored-by: Bluenix <bluenixdev@gmail.com>

Bot: update comment on joining threads

fe12eaa

Co-authored-by: Bluenix <bluenixdev@gmail.com>

Bot: remove inaccurate docstrings

b61a019

Akarys42 requested a review from Bluenix2 September 19, 2021 13:26

Bluenix2 reviewed Sep 19, 2021

View reviewed changes

Modlog: change thread logging to past-tense

9b82abd

Co-authored-by: Bluenix <bluenixdev@gmail.com>

Akarys42 requested a review from Bluenix2 September 21, 2021 07:40

Merge branch 'main' into discord-2.0

c050ac5

HassanAbouelela previously requested changes Oct 10, 2021

View reviewed changes

jb3 added the review: needs devops label Oct 10, 2021

jb3 mentioned this pull request Oct 10, 2021

!subscribe adjustments to allow for sub-roles python-discord/meta#150

Closed

janine9vn mentioned this pull request Oct 10, 2021

Change !subscribe to Buttons for sub-roles #1866

Closed

Akarys42 added review: do not merge The PR can be reviewed but cannot be merged now and removed review: needs devops labels Oct 11, 2021

ChrisLovering mentioned this pull request Oct 12, 2021

Subscribe with buttons #1868

Merged

Merge branch 'main' into discord-2.0

bff6d4b

brad90four approved these changes Oct 13, 2021

View reviewed changes

ChrisLovering mentioned this pull request Oct 14, 2021

Modlog ignore channels mods can't view #1871

Merged

ChrisLovering requested changes Oct 14, 2021

View reviewed changes

Akarys42 added 2 commits October 15, 2021 11:37

Defcon: remove tzinfo from member.created_at

a791dac

Voice gate: set tzinfo to None in author.joined_at

574f610

Akarys42 requested a review from ChrisLovering October 15, 2021 09:39

ChrisLovering approved these changes Oct 15, 2021

View reviewed changes

Merge branch 'main' into discord-2.0

7635794

Akarys42 force-pushed the discord-2.0 branch from a7587cf to 7635794 Compare October 15, 2021 09:56

Merge remote-tracking branch 'origin/main' into discord-2.0

992507b

Akarys42 removed the review: do not merge The PR can be reviewed but cannot be merged now label Oct 17, 2021

Akarys42 merged commit 4050b0c into main Oct 17, 2021

Akarys42 deleted the discord-2.0 branch October 17, 2021 08:16

Akarys42 mentioned this pull request Oct 17, 2021

Refactor time utilities #1721

Merged

mbaruh mentioned this pull request Oct 19, 2021

Move to timezone aware datetimes #1895

Merged

Xithrius removed the s: needs review Author is waiting for someone to review and approve label Nov 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate to Discord 2.0a0 #1815

Migrate to Discord 2.0a0 #1815

Akarys42 commented Sep 6, 2021 •

edited

Kronifer left a comment

Bluenix2 left a comment

Bluenix2 Sep 17, 2021

mbaruh Sep 18, 2021

mbaruh Sep 18, 2021

Bluenix2 Sep 18, 2021

mbaruh Sep 19, 2021

Akarys42 Sep 19, 2021

mbaruh Sep 19, 2021

Bluenix2 Sep 18, 2021

Akarys42 Sep 19, 2021 •

edited

Bluenix2 Sep 19, 2021

Akarys42 Sep 21, 2021

ChrisLovering Sep 21, 2021 •

edited

Bluenix2 Sep 21, 2021

Akarys42 Sep 21, 2021

Bluenix2 Sep 21, 2021

Akarys42 Oct 5, 2021

Bluenix2 Sep 19, 2021

Akarys42 Sep 21, 2021

HassanAbouelela left a comment

brad90four left a comment

ChrisLovering left a comment •

edited

Akarys42 commented Oct 15, 2021

ChrisLovering commented Oct 15, 2021

ChrisLovering left a comment

Migrate to Discord 2.0a0 #1815

Migrate to Discord 2.0a0 #1815

Conversation

Akarys42 commented Sep 6, 2021 • edited

Kronifer left a comment

Choose a reason for hiding this comment

Bluenix2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Akarys42 Sep 19, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrisLovering Sep 21, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HassanAbouelela left a comment

Choose a reason for hiding this comment

brad90four left a comment

Choose a reason for hiding this comment

ChrisLovering left a comment • edited

Choose a reason for hiding this comment

Akarys42 commented Oct 15, 2021

ChrisLovering commented Oct 15, 2021

ChrisLovering left a comment

Choose a reason for hiding this comment

Akarys42 commented Sep 6, 2021 •

edited

Akarys42 Sep 19, 2021 •

edited

ChrisLovering Sep 21, 2021 •

edited

ChrisLovering left a comment •

edited