dev-log

[regal arch]

aeros

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

I'm not sure that I agree with the usage of assertIs() in this case rather than assertEqual(). Although in this case the result is the same, you typically want to use assertIs() to specifically convey that you're comparing object identity, which isn't relevant in this case.

aeros

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

aeros

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

I think we should expand upon the subtests to include user IDs that follow a more realistic format, perhaps using the real user IDs of a few staff members (since it's fully public information). Also, having something around 5 valid user IDs and 5 invalid ones would help to improve coverage (not literal code coverage %; I'm referring to coverage of possible inputs).

IMO, the ones might be a bit too unrealistic for testing purposes.

aeros

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

aeros

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

With there now being two separate ways to get a False return from is_maybe_token, we may want to consider adding some logging to clearly separate each case for debugging purposes. I'd also change it to an explicit else clause instead of the bare return False. For example:

    @classmethod
    def is_maybe_token(cls, test_str: str) -> bool:
        """Check the provided string to see if it is a seemingly valid token."""
        try:
            user_id, creation_timestamp...

[regal arch]

aeros

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

aeros

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

Also, this may be more of a personal preference, but I've found that it's significantly easier to read and debug when the valid and invalid cases are separated into 2+ tests, and then using something like assertTrue(result) and assertFalse(result) for each of the subtests respectively. At a glance, it can sometimes be needlessly difficult to discern whether it should be true or false for that specific input, especially if the list of test inputs grow.

aeros

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

aeros

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

What exactly is the purpose of this specific test being a coroutine method? It's not immediately clear to me.

(FYI: you can have non coroutine test methods defined in an IsolatedAsyncioTestCase class)

[regal arch]

Aaryan-K

[python-discord/seasonalbot] New comment on issue #174: \[Feature Request\] Earth Photos

I'm interested in trying to do this

aeros

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

Could mod_log not just use the @autospec decorator since it allows any valid **kwargs for patch.object()?

E.g. @autospec(TokenRemover, "mod_log", new_callable=mock.PropertyMock) instead of having @mock.patch.object(TokenRemover, "mod_log", new_callable=mock.PropertyMock) and mod_log = mock.create_autospec(ModLog, spec_set=True, instance=True)? It would behave the same AFAICT, but it seems worthwhile to remove a line of boilerplate.

Note that I don't personally have much ex...

aeros

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

MarkKoz

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

MarkKoz

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

new_callable cannot be used if autospeccing. I could set autospec=False but at that point I felt like I should use the normal unittest decorators.

MarkKoz

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

MarkKoz

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

No reason any more. Forgot to remove it.

MarkKoz

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

MarkKoz

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

Well you wouldn't consider if x == True: correct, right? So why would assertEqual be correct when comparing booleans? Sure it works but I've learned it's not good practice to compare booleans and None with ==.

MarkKoz

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

MarkKoz

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

I can get 5 valid ones but I don't know would be considered a "realistic" invalid value.

MarkKoz

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

MarkKoz

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

I agree on the readability point but I didn't feel it was worth splitting up given the low amount of test inputs and the simplicity of the test. If more inputs are added like you suggested, then I'll probably split it up.

MarkKoz

[bot:bug/filters/928/non\-ascii\-token] 1 new commit

31aff51 Fix a test needlessly being a coroutine - MarkKoz

[ebon magnet]

Build 20200514.1 succeeded

Requested by

GitHub

Duration

00:01:25

Build pipeline

Bot

[regal arch]

MarkKoz

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

MarkKoz

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

Well, I suppose. This was intended to be a quick fix so I went with catching the exception.

MarkKoz

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

MarkKoz

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

Good catch. I will change this.

MarkKoz

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

MarkKoz

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

I agree.

MarkKoz

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

MarkKoz

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

Good catch. Also, the channel name is already being formatted (it uses channel.mention).

MarkKoz

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

MarkKoz

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

Do you think regex would be clearer, isdecimal() and isascii() (not sure if that's right even, the character classes are confusing), or something comparing ordinal range for ASCII digits?

MarkKoz

[bot:bug/filters/928/non\-ascii\-token] 1 new commit

ab44bb3 Add missing comma to token remover log message - MarkKoz

[ebon magnet]

Build 20200514.2 succeeded

Requested by

GitHub

Duration

00:01:23

Build pipeline

Bot

[regal arch]

MarkKoz

[bot:bug/filters/928/non\-ascii\-token] 1 new commit

297089c Token remover: add logs to clarify why token is... - MarkKoz

[ebon magnet]

Build 20200514.3 succeeded

Requested by

GitHub

Duration

00:01:19

Build pipeline

Bot

[regal arch]

MarkKoz

[python-discord/bot] New comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

I'm not sure how the is_valid_timestamp works as I couldn't find anything about how the token structure works but it doesn't seem to be behaving as expected.

With the int struct unpacking, the resulting number can be negative which just seems invalid if it's a timestamp.
Then the "snowflake" + TOKEN_EPOCH gets passed to snowflake_time where the value gets right shifted into some small number and a datetime is returned from that, as that's supposed to work with the ...

[ebon magnet]

Build 20200514.4 failed

Requested by

GitHub

Duration

00:00:55

Build pipeline

Bot

[north knot]

Postgres backup completed!

[ebon magnet]

Build 20200514.5 succeeded

Requested by

GitHub

Duration

00:01:24

Build pipeline

Bot

[regal arch]

ks129

[python-discord/bot] New comment on pull request #901: PEP command improvisations

@Numerlor Is this better now?

[regal arch]

SebastiaanZ

[bot:master] 1 new commit

8d59546 Remove everyone-ping from mentions alert - SebastiaanZ

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200514.6 succeeded

Requested by

GitHub

Duration

00:03:05

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

lemonsaurus

[bot:master] 1 new commit

4480e1d Remove @Admins ping from the #verification message - lemonsaurus

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200514.7 succeeded

Requested by

Leon Sandøy

Duration

00:03:22

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

Numerlor

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

Numerlor

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

The bot logs will only include <#ID> which is less useful than a name with the id but I guess that's worth the tradeoff because then it'd result in the name being logged twice in the channel

[regal arch]

itsdrike

[python-discord/bot] Fork created: itsdrike/bot

Numerlor

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

Numerlor

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

Checking isdecimal and isascii should be clear while remaining faster than regex or validating it manually.
With the number string methods and isascii, only 0-9 will match

Numerlor

[python-discord/bot] New comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

2. After the token epoch is added, we already have our Unix timestamp; it's wrong to interpret it as a snowflake

From my tests it looks like the decoded int is directly a valid timestamp, without adding the token epoch as shown above. I think int.from_bytes can also be used instead of the struct unpacking to be a bit clearer

[regal arch]

aeros

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

aeros

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

Well you wouldn't consider if x == True: correct, right? So why would assertEqual be correct when comparing booleans? Sure it works but I've learned it's not good practice to compare booleans and None with ==.

Yeah, disregard that comment; you're correct. It's actually better to use assertIs() over assertEqual() when directly testing a boolean return, since the truthiness of the object (from bool(object)) could give rather misleading results if the function returns anything other t...

aeros

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

aeros

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

That's fine. When I mentioned realistic inputs, I was mainly referring to the valid ones. For the invalid inputs, they should just mainly have some variety.

[regal arch]

ks129

[python-discord/bot] Pull request opened: #942 Python News: Implement stats

Add stat increaser to PEP and maillist posting.

This got approved in #meta channel. Very small change so no need for an issue I think.

[ebon magnet]

Build 20200514.9 succeeded

Requested by

GitHub

Duration

00:01:26

Build pipeline

Bot

[regal arch]

ikuyarihS

[python-discord/bot] Pull request review submitted: #942 Python News: Implement stats

ikuyarihS

[python-discord/bot] Pull request review submitted: #941 Add message publishing to Reddit cog

ikuyarihS

[python-discord/bot] Pull request review submitted: #866 feature to restrict tags to specific role\(s\)

ikuyarihS

[python-discord/bot] New comment on pull request #866: feature to restrict tags to specific role\(s\)

There's a minor config, can you resolve it @RohanJnr then it's good to go!

ikuyarihS

[python-discord/bot] Pull request review submitted: #858 Add mutability\.md tag

ikuyarihS

[python-discord/bot] New review comment on pull request #858: Add mutability\.md tag

Also, should string be renamed to text instead, to avoid shadowing the string module?

[ebon magnet]

Build 20200514.10 failed

Requested by

GitHub

Duration

00:01:17

Build pipeline

Bot

[ebon magnet]

Build 20200514.11 succeeded

Requested by

GitHub

Duration

00:01:20

Build pipeline

Bot

[regal arch]

Numerlor

[python-discord/bot] Issue opened: #943 Include more text formats in the antimalware filter

The #925 PR got merged recently, but only handles .txt files;
this leaves out other common shared file types like .csv to be handled using the generic filter message, which doesn't mention the paste service as an alternative to direct file upload and sometimes results in people circumventing the filter.

[regal arch]

kwzrd

[python-discord/bot] Pull request review submitted: #858 Add mutability\.md tag

kwzrd

[python-discord/bot] New review comment on pull request #858: Add mutability\.md tag

I agree, this would be less confusing.

[regal arch]

MarkKoz

[bot:bug/filters/928/non\-ascii\-token] 1 new commit

73bcb2b Token remover: fix timestamp check - MarkKoz

[ebon magnet]

Build 20200514.12 failed

Requested by

GitHub

Duration

00:00:57

Build pipeline

Bot

[regal arch]

Numerlor

[python-discord/bot] Pull request review submitted: #901 PEP command improvisations

Not a fan of passing the bool flag (or recreating it with pep 0) from the get_pep_embed method.
This could be avoided by moving the actual fetching of the pep from github into an another function, and then handle the sending and error Embed creation in a single method.

Numerlor

[python-discord/bot] New review comment on pull request #901: PEP command improvisations

The docstring no longer describes the current behavior of the function.

Numerlor

[python-discord/bot] New review comment on pull request #901: PEP command improvisations

While there's the chance, this can be changed to use the prevalent indent style of the repo

Numerlor

[python-discord/bot] New review comment on pull request #901: PEP command improvisations

The typehint should be in the assignment line, but I don't feel like it's needed at all in this context

[north knot]

Postgres backup completed!

[ebon magnet]

Build 20200515.1 succeeded

Requested by

GitHub

Duration

00:01:52

Build pipeline

Bot

[regal arch]

ks129

[python-discord/bot] Pull request review submitted: #901 PEP command improvisations

ks129

[python-discord/bot] New review comment on pull request #901: PEP command improvisations

Fixed in 36dac33 . I used this for PyCharm auto-suggesting and bug finding, but forget to remove it.

ks129

[python-discord/bot] Pull request review submitted: #901 PEP command improvisations

ks129

[python-discord/bot] New review comment on pull request #901: PEP command improvisations

Fixed in 248ce24

ks129

[python-discord/bot] New comment on pull request #901: PEP command improvisations

@Numerlor Is the current way is better?

[regal arch]

Py-Ryan

[python-discord/bot] Fork created: Py-Ryan/bot

[regal arch]

kosayoda

[python-discord/bot] Pull request review dismissed: #519 Refactor the \!help command\.

Okay I ran out of time for reviewing this now so more to come, good work though

[ebon magnet]

Build 20200515.2 succeeded

Requested by

GitHub

Duration

00:01:36

Build pipeline

Bot

[regal arch]

lemonsaurus

[python-discord/bot] Pull request review submitted: #519 Refactor the \!help command\.

Apply suggestions from Mark's code review. is a commit message that doesn't really tell me anything. It'd be better to actually list what the changes were made.

Overall, though. this looks pretty good. I didn't test this, but @SebastiaanZ will do a test and a quick review of the changes to the redirect_output decorator, so don't merge this until he approves.

lemonsaurus

[python-discord/bot] New review comment on pull request #519: Refactor the \!help command\.

Don't do this. Trailing commas are good design, because if we were to add another kwarg we only have to change a single line instead of two, which ensures correct git blame is maintained.

lemonsaurus

[python-discord/bot] New review comment on pull request #519: Refactor the \!help command\.

I also hate fmt as a variable name.

can you also just go through this file and remove other instances of ninja code?

lemonsaurus

[python-discord/bot] New review comment on pull request #519: Refactor the \!help command\.

Can you explain why this decorator is here and not decorating for example command_callback? It feels strange to me that it decorates a function that does not output anything.

lemonsaurus

[python-discord/bot] Pull request review submitted: #519 Refactor the \!help command\.

lemonsaurus

[python-discord/bot] New review comment on pull request #519: Refactor the \!help command\.

Seems like Ves will address this better in his review, I'll resolve it here for now.

[regal arch]

SebastiaanZ

[python-discord/bot] New comment on issue #898: Prevent a help channel from going dormant if a user is typing in it

1. this should be a feature
2. there's a reasonable way to implement it.

I like the feature, but I'm less sure about the implementation.

whenever on_typing is triggered, if there's less than 5 minutes left, we set the timeout to 5 minutes. that should keep it from timing out if someone is working on a long message.

If a Task is currently pending because it's awaiting an asyncio.sleep, is there an easy way to introspect the time that's left? I think we'll have to investigate...

[regal arch]

Akarys42

[python-discord/bot] New comment on issue #898: Prevent a help channel from going dormant if a user is typing in it

Switching to a scheduler instead of asyncio.sleep calls would be the smartest here IMO, you could simply reschedule the deletion task of 5 minutes if someone is typing, plus you can know if there's less than 5 minutes less.

SebastiaanZ

[python-discord/bot] New comment on issue #898: Prevent a help channel from going dormant if a user is typing in it

I think that would mean a lot of rescheduling if people start having a conversation in those last five minutes. Each typing event would reschedule the task for just a few seconds. If you keep track of typing event times, you reschedule a maximum of once per five minutes and keeping track of a time int is minimal work.

Akarys42

[python-discord/bot] New comment on issue #898: Prevent a help channel from going dormant if a user is typing in it

Well, we could do so if someone types during the last 4 minutes, it schedules back to 6 minutes or so. Re-sleeping, when the timer runs out, sounds a bit messy IMO, but I guess that works :grin:.

[regal arch]

MarkKoz

[bot:bug/filters/928/non\-ascii\-token] 1 new commit

4a73c24 Token remover: use strict check for digits in t... - MarkKoz

SebastiaanZ

[python-discord/bot] New comment on issue #898: Prevent a help channel from going dormant if a user is typing in it

I don't see why we'd need the added complexity and added work.

Our on_typing could be as simple as

@Cog.listener()
async def typing(self, channel, user, when):
    if in_category(channel.id, Category.in_use):
        self._last_typing_event[channel.id] = when

The comparison in the time-out deletion task would be trivial with datetime.datetime.now() and datetime.timedelta, and the channel move_to_dormant only needs to delete a key from that dict (even we even care...

[ebon magnet]

Build 20200515.3 failed

Requested by

GitHub

Duration

00:01:05

Build pipeline

Bot

[regal arch]

Numerlor

[python-discord/bot] Pull request opened: #944 Increase snekbox re eval timeout to 30 seconds

Increases the re eval message edit timeout to 30 seconds as discussed in #920

[ebon magnet]

Build 20200515.4 failed

Requested by

GitHub

Duration

00:00:54

Build pipeline

Bot

Build 20200515.5 succeeded

Requested by

GitHub

Duration

00:01:33

Build pipeline

Bot

[regal arch]

SebastiaanZ

[python-discord/bot] Pull request review submitted: #519 Refactor the \!help command\.

Hey!

I think your PR has uncovered a fundamental flaw in our redirect_output decorator, but I'm not sure that the way in which you've currently solved it is the best way. The steps you took to mitigate the issue you encountered in your !help-command implementation require tweaks both at the site of the decorator (scheduling rather than awaiting the invocation deletion steps) and the command itself (moving the decorator to a dummy helper function that returns early and is only there to ...

[regal arch]

mathsman5133

[python-discord/bot] New comment on pull request #519: Refactor the \!help command\.

OK pretty sure everything mentioned has been fixed. Pretty sure I got all the ninja code but please lmk if I missed something.

Apply suggestions from Mark's code review. is a commit message that doesn't really tell me anything. It'd be better to actually list which changes were made.
Regarding this, I did try to rebase and fix this message but either I'm still useless with git or reading from 3 months ago didn't go down too well and it tried to get me to manually choose/merge every com...

[north knot]

Postgres backup completed!

[regal arch]

Savant-Dev

[python-discord/bot] New comment on issue #943: Include more text formats in the antimalware filter

The inclusion of .csv, .json and like file-types was discussed heavily in #meta around the time #925 was being solved. The consensus was that the primary purpose of that PR was to fix Discord's auto-conversion of long messages (2000 chars >)

Perhaps, instead of trying to include other file-types in the .txt filter you could simply revise the existing embed under the else to include the paste link?
![Screenshot from 2020-05-16 02-31-08](https://user-images.githubusercontent.com/64...

[regal arch]

Numerlor

[python-discord/bot] New comment on issue #943: Include more text formats in the antimalware filter

Including it in the geneic embed wouldn't be helpful for the binary types the filter is intended for so I don't think it's worth putting it there.

The discussions I remember about csv etc. was about excluding the from the filters completely; not about adding them to the new handler. The original issue did also talk about a list of text extensions

[regal arch]

vivax3794

[python-discord/seasonalbot] Pull request closed: #389 Wikipedia command

[regal arch]

ks129

[python-discord/bot] Pull request opened: #945 Add more statistics

Added following new statistics:

• Guild boost statistics
• Markdown code block correction
• eval fails and successes
• Who runs eval? Helper or Developer?
• Where eval run? Topical, help channels or bot commands

There is no public issue, but one private organization issue.

[ebon magnet]

Build 20200516.1 succeeded

Requested by

GitHub

Duration

00:01:20

Build pipeline

Bot

[regal arch]

fiskenslakt

[python-discord/bot] Pull request review submitted: #934 Filtering: don't delete messages in DMs

Looks good to me.

[regal arch]

vivax3794

[python-discord/bot] Fork created: vivax3794/bot

vivax3794

[python-discord/bot] Fork created: vivax3794/bot

vivax3794

[python-discord/bot] Pull request opened: #946 added "solved" as a alias for "closed"

I have seen many users try !solved

So, why not add it as a command alias to make it easier for them :smile:

[ebon magnet]

Build 20200516.2 succeeded

Requested by

GitHub

Duration

00:01:09

Build pipeline

Bot

[regal arch]

lemonsaurus

[python-discord/bot] Pull request review submitted: #946 add "solved" as a alias for "closed"

why not.

lemonsaurus

[python-discord/bot] Pull request closed: #946 add "solved" as a alias for "closed"

lemonsaurus

[bot:master] 2 new commits

3a278c7 added "solved" as a alias for "closed" - vivax3794
175f7ef Merge pull request #946 from vivax3794/master - lemonsaurus

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200516.3 succeeded

Requested by

Leon Sandøy

Duration

00:03:20

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

lemonsaurus

[bot:redis\_persistence] 11 new commits

3aeb9e6 Adding redis to docker-compose file. - lemonsaurus
5382dd8 Adding redis-py to the Pipfile - lemonsaurus
7a501fd Boilerplate for the RedisCacheMixin - lemonsaurus
588521c Refactor - no more mixins! - lemonsaurus
ee8386e Add basic dict methods for RedisDict. - lemonsaurus

lemonsaurus

[python-discord/bot] New branch created: redis\_persistence

lemonsaurus

[bot:redis\_persistence] 1 new commit

258ad9d Make redis host and port configurable. - lemonsaurus

lemonsaurus

[bot:redis\_persistence] 1 new commit

8456023 namespace "general" -> "global" - lemonsaurus

[regal arch]

lemonsaurus

[python-discord/bot] Pull request opened: #947 RedisDict \- Data Persistence

This pull requests adds a new feature that the bot can use to add persistant storage.

This works by utilizing a Redis server as a cache, while providing a user-friendly dict-like interface.

Let's have a look at how it works and exactly what this pull request contains.

Creating a new RedisDict

The star of the show is the RedisDict. To create a new one of these, you have two options.

# Either create it as a class attribute
class SomeClass:
    my_cache = RedisDict()

...

[ebon magnet]

Build 20200517.1 failed

Requested by

GitHub

Duration

00:01:30

Build pipeline

Bot

[regal arch]

lemonsaurus

[bot:redis\_persistence] 3 new commits

3a278c7 added "solved" as a alias for "closed" - vivax3794
175f7ef Merge pull request #946 from vivax3794/master - lemonsaurus
f19c38c Merge branch 'master' into redis_persistence - lemonsaurus

[ebon magnet]

Build 20200517.2 failed

Requested by

GitHub

Duration

00:01:31

Build pipeline

Bot

[regal arch]

MarkKoz

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

MarkKoz

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

Sorry, you're right. I think I'll leave it alone though. I don't see a clean way to have separate messages and it doesn't feel to be worth the trouble.

[regal arch]

MarkKoz

[bot:bug/filters/928/non\-ascii\-token] 1 new commit

ad154f7 Add a utility function to pad base64 data - MarkKoz

[ebon magnet]

Build 20200517.3 failed

Requested by

GitHub

Duration

00:01:08

Build pipeline

Bot

[north knot]

Postgres backup completed!

[regal arch]

MarkKoz

[python-discord/bot] Pull request review submitted: #901 PEP command improvisations

Just some comments for now. Still need to test locally.

MarkKoz

[python-discord/bot] New review comment on pull request #901: PEP command improvisations

If you're not actually using the value, use in instead to check if it's in the dict.

            if key in async_cache.cache:

MarkKoz

[python-discord/bot] New review comment on pull request #901: PEP command improvisations

Please avoid putting newlines in logs; it makes them harder to read.

MarkKoz

[python-discord/bot] New review comment on pull request #901: PEP command improvisations

            embed=Embed(title="PEP not found", description=not_found, colour=Colour.red())
            await ctx.send(embed=embed)

[ebon magnet]

Build 20200517.4 succeeded

Requested by

GitHub

Duration

00:01:22

Build pipeline

Bot

Build 20200517.5 succeeded

Requested by

GitHub

Duration

00:01:23

Build pipeline

Bot

[ebon magnet]

Build 20200517.6 succeeded

Requested by

GitHub

Duration

00:01:25

Build pipeline

Bot

[regal arch]

lemonsaurus

[python-discord/bot] New comment on pull request #519: Refactor the \!help command\.

@mathsman5133 no need to rebase to fix commits. those comments are basically just for future reference.

SebastiaanZ

[python-discord/bot] Pull request review submitted: #519 Refactor the \!help command\.

SebastiaanZ

[python-discord/bot] Issue closed: #502 Refactor "Help" Command

SebastiaanZ

[python-discord/bot] Issue closed: #100 Wolfram custom cooldowns being triggered on help output

SebastiaanZ

[python-discord/bot] Pull request closed: #519 Refactor the \!help command\.

SebastiaanZ

[bot:master] 19 new commits

05616ae Refactor the Help command. - mathsman5133
87f9fdd Minor formatting changes to align with current ... - mathsman5133
5ebb87d Add a special case for when the help command in... - mathsman5133
79d5b2d Few changes to keep formatting same as current - mathsman5133
d8384b2 Show a maximum of 8 commands per page rather th... - mathsman5133

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200517.7 succeeded

Requested by

GitHub

Duration

00:03:06

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[ebon magnet]

Build 20200517.8 succeeded

Requested by

GitHub

Duration

00:01:14

Build pipeline

Bot

Build 20200517.9 succeeded

Requested by

GitHub

Duration

00:01:14

Build pipeline

Bot

[ebon magnet]

Build 20200517.10 succeeded

Requested by

GitHub

Duration

00:01:16

Build pipeline

Bot

[regal arch]

ks129

[python-discord/bot] New comment on pull request #901: PEP command improvisations

@MarkKoz Addressed changes.

[regal arch]

lemonsaurus

[bot:redis\_persistence] 1 new commit

e993566 Fix linting errors introduced by flake8 3.8 - lemonsaurus

lemonsaurus

[python-discord/bot] New comment on issue #884: Update pyflakes to version 2\.2\.0

flake8 version will be bumped to 3.8 in #947

[ebon magnet]

Build 20200517.11 succeeded

Requested by

GitHub

Duration

00:02:23

Build pipeline

Bot

[regal arch]

lemonsaurus

[bot:redis\_persistence] 20 new commits

05616ae Refactor the Help command. - mathsman5133
87f9fdd Minor formatting changes to align with current ... - mathsman5133
5ebb87d Add a special case for when the help command in... - mathsman5133
79d5b2d Few changes to keep formatting same as current - mathsman5133
d8384b2 Show a maximum of 8 commands per page rather th... - mathsman5133

[ebon magnet]

Build 20200517.12 succeeded

Requested by

GitHub

Duration

00:01:20

Build pipeline

Bot

[regal arch]

SebastiaanZ

[bot:help\-command\-fix\-invocation] 1 new commit

f73489f Use send_help to invoke command help - SebastiaanZ

SebastiaanZ

[python-discord/bot] New branch created: help\-command\-fix\-invocation

sentry-io[bot]

[python-discord/bot] Issue opened: #948 AttributeError: 'Help' object has no attribute 'channel'

Currently, we're invoking our help command manually with:

await ctx.invoke(self.bot.get_command("help"), "bot")

Unfortunately, this fails after the recent refactor of the help command. The fix is to use ctx.send_help(command.qualified_name) instead.

---

Sentry Issue: BOT-4T

AttributeError: 'Help' object has no attribute 'channel'
  File "discord/ext/commands/core.py", line 83,...

SebastiaanZ

[python-discord/bot] Pull request opened: #949 Use \`send\_help\` to ensure that our help command is correctly invoked

Currently, we're invoking our help command manually like this:

await ctx.invoke(self.bot.get_command("help"), "bot")

However, unfortunately, this fails when the command callback function is decorated, leading to exceptions like:

AttributeError: 'Help' object has no attribute 'channel'
  File "discord/ext/commands/core.py", line 83, in wrapped
    ret = await coro(*args, **kwargs)
  File "bot/cogs/snekbox.py", line 292, in eval_command
    await ctx.invoke(self....

[ebon magnet]

Build 20200517.13 failed

Requested by

GitHub

Duration

00:01:02

Build pipeline

Bot

[regal arch]

SebastiaanZ

[bot] Branch help\-command\-fix\-invocation was force-pushed to `4e24e9c`

Compare changes

mathsman5133

[python-discord/bot] New comment on pull request #949: Use \`send\_help\` to ensure that our help command is correctly invoked

A Quick comment, would it be more efficient to pass in ctx.command rather than the name of the command, where applicable? Passing in a string means an extra lookup and if for some reason the command name were to change this way means an additional change isn't required

[ebon magnet]

Build 20200517.14 succeeded

Requested by

GitHub

Duration

00:01:21

Build pipeline

Bot

[regal arch]

SebastiaanZ

[python-discord/bot] New comment on pull request #949: Use \`send\_help\` to ensure that our help command is correctly invoked

A Quick comment, would it be more efficient to pass in ctx.command rather than the name of the command, where applicable? Passing in a string means an extra lookup and if for some reason the command name were to change this way means an additional change isn't required

The docs don't mention support for passing in a Context object. It can either be a Cog, a Command-object, or a string.

decorator-factory

[bot:decorator\-factory\-mutability\-tag] 1 new commit

a983d49 Apply language improvements proposed from kwzrd - decorator-factory

mathsman5133

[python-discord/bot] New comment on pull request #949: Use \`send\_help\` to ensure that our help command is correctly invoked

Right, but can't you get a Command object with ctx.command?

decorator-factory

[bot:decorator\-factory\-mutability\-tag] 1 new commit

1db4fe2 Change standalone programs to interactive sessions - decorator-factory

[ebon magnet]

Build 20200517.15 succeeded

Requested by

GitHub

Duration

00:01:10

Build pipeline

Bot

Build 20200517.16 succeeded

Requested by

GitHub

Duration

00:01:18

Build pipeline

Bot

[regal arch]

decorator-factory

[bot:decorator\-factory\-mutability\-tag] 1 new commit

abb9e57 Add a note on user-defined classes - decorator-factory

[ebon magnet]

Build 20200517.17 succeeded

Requested by

GitHub

Duration

00:01:10

Build pipeline

Bot

[regal arch]

SebastiaanZ

[python-discord/bot] New comment on pull request #949: Use \`send\_help\` to ensure that our help command is correctly invoked

I accidentally hit edit instead of reply.

SebastiaanZ

[bot:help\-command\-fix\-invocation] 1 new commit

87cec1a Use Command-object for send_help - SebastiaanZ

[ebon magnet]

Build 20200517.18 succeeded

Requested by

GitHub

Duration

00:01:21

Build pipeline

Bot

[regal arch]

lemonsaurus

[python-discord/bot] Pull request review submitted: #949 Use \`send\_help\` to ensure that our help command is correctly invoked

Yes. This change is an improvement anyway, the previous approach was fighting the framework.

kwzrd

[python-discord/bot] Pull request review submitted: #949 Use \`send\_help\` to ensure that our help command is correctly invoked

Appears functional and I agree that this is an overall improvement.

kwzrd

[python-discord/bot] Pull request closed: #949 Use \`send\_help\` to ensure that our help command is correctly invoked

kwzrd

[python-discord/bot] branch deleted: help\-command\-fix\-invocation

kwzrd

[bot:master] 3 new commits

4e24e9c Use send_help to invoke command help - SebastiaanZ
87cec1a Use Command-object for send_help - SebastiaanZ
65b02cd Merge pull request #949 from python-discord/hel... - kwzrd

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200517.19 succeeded

Requested by

GitHub

Duration

00:03:00

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

decorator-factory

[python-discord/bot] Pull request review submitted: #858 Add mutability\.md tag

decorator-factory

[python-discord/bot] New review comment on pull request #858: Add mutability\.md tag

changed

SebastiaanZ

[python-discord/bot] New comment on issue #948: AttributeError: 'Help' object has no attribute 'channel'

Fixed by #949

SebastiaanZ

[python-discord/bot] Issue closed: #948 AttributeError: 'Help' object has no attribute 'channel'

kwzrd

[python-discord/bot] Pull request review submitted: #858 Add mutability\.md tag

Thanks for addressing the feedback, @decorator-factory. I think this looks really good now:

@ikuyarihS made a good comment earlier that the string variable in the examples could be renamed to text or similar to prevent collision with the string stdlib module (and also maybe it would make some parts of the tag more clear, where it can be difficult to tell whether we're ref...

decorator-factory

[bot:decorator\-factory\-mutability\-tag] 1 new commit

4f76259 Rename string to greeting - decorator-factory

[ebon magnet]

Build 20200517.20 succeeded

Requested by

GitHub

Duration

00:01:23

Build pipeline

Bot

[regal arch]

decorator-factory

[bot:decorator\-factory\-mutability\-tag] 1 new commit

d390fb2 Fix incomplete variable renaming - decorator-factory

kwzrd

[python-discord/bot] Pull request review submitted: #858 Add mutability\.md tag

Looks excellent, thank you for handling this!

kwzrd

[bot:decorator\-factory\-mutability\-tag] 339 new commits

05616ae Refactor the Help command. - mathsman5133
87f9fdd Minor formatting changes to align with current ... - mathsman5133
5ebb87d Add a special case for when the help command in... - mathsman5133
79d5b2d Few changes to keep formatting same as current - mathsman5133
d8384b2 Show a maximum of 8 commands per page rather th... - mathsman5133

[ebon magnet]

Build 20200517.22 succeeded

Requested by

GitHub

Duration

00:01:16

Build pipeline

Bot

[regal arch]

kwzrd

[python-discord/bot] Pull request closed: #858 Add mutability\.md tag

kwzrd

[bot:master] 10 new commits

2559bc9 Add mutability.md tag - decorator-factory
028781a header->bold in mutability.md - decorator-factory
a946842 Fix hard-wrapping in mutability.md - decorator-factory
a983d49 Apply language improvements proposed from kwzrd - decorator-factory
1db4fe2 Change standalone programs to interactive sessions - decorator-factory

kwzrd

[python-discord/bot] branch deleted: decorator\-factory\-mutability\-tag

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200517.23 succeeded

Requested by

GitHub

Duration

00:02:57

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[ebon magnet]

Build 20200517.24 succeeded

Requested by

GitHub

Duration

00:01:16

Build pipeline

Bot

[regal arch]

ks129

[python-discord/bot] New comment on pull request #860: discord\.py 1\.3\.x Functionality Migrating

@MarkKoz Now PR that blocked this is merged.

ks129

[python-discord/bot] Pull request opened: #950 Created tests for startup logging

The title says all. Closes #583

[ebon magnet]

Build 20200517.26 succeeded

Requested by

GitHub

Duration

00:01:15

Build pipeline

Bot

[regal arch]

kwzrd

[python-discord/bot] Pull request closed: #947 RedisDict \- Data Persistence

kwzrd

[python-discord/bot] Pull request reopened: #947 RedisDict \- Data Persistence

Akarys42

[python-discord/bot] Pull request opened: #951 Allow commands in \#seasonalbot\-commands

While chatting in #seasonalbot-commands yesterday, I tried to set a reminder to open an issue about the bot, but this channel is marked as allowed in the bot code. This PR allows commands to be executed everywhere #bot-commands was already allowed.

[ebon magnet]

Build 20200517.27 succeeded

Requested by

GitHub

Duration

00:01:17

Build pipeline

Bot

[regal arch]

SebastiaanZ

[python-discord/bot] New comment on pull request #951: Allow commands in \#seasonalbot\-commands

We made a conscious decision not to allow bot-commands in seasonalbot-commands. The seasonalbot-commands channel is the only channel that showcases SeasonalBot and its features and we already have a dedicated bot-commands channel.

SebastiaanZ

[python-discord/bot] Pull request closed: #951 Allow commands in \#seasonalbot\-commands

ks129

[python-discord/bot] New comment on issue #575: Write unit tests for \`bot/cogs/error\_handler\.py\`

Can I get assigned to this?

Akarys42

[python-discord/seasonalbot] Pull request opened: #408 Use input hashing instead of randomness inside 8ball

Description

This PR make the .8ball command use hashing instead of random.choice()

Reasoning

This makes the outcome always the same for the same question, so a user cannot brute-force the 8ball until they obtain the answer they want :grin:.

Did you:

• [X] Join the Python Discord Community?
• [X] If dependencies have been added or updated, run pipenv lock?
• [X] Lint your code (pipenv run lint)?
• [X] Set the PR t...

[ebon magnet]

Build 20200517.1 succeeded

Requested by

GitHub

Duration

00:01:00

Build pipeline

Seasonal Bot

[regal arch]

Numerlor

[python-discord/bot] New comment on issue #920: Increase snekbox re\-eval timeout

I've opened a PR to increase the timeout while a more noticeable and clearer interface is agreed upon here; but as @SebastiaanZ pointed out in #dev-contrib there's really no downside to increasing the timeout beyond the 30s suggested here.

Any thoughts on a longer timeout? Two minutes were proposed in the #dev-contrib convo but I feel like that may be a bit long if the code would be re-evaled at the end of the timeout when a channel is a bit more active; although that shouldn't happen that...

kwzrd

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

I haven't tested yet, but I wanted to leave a couple of initial comments and questions. I have no experience with Redis so this is a learning opportunity for me.

It has taken me some time to figure out how the namespace is inferred when not provided explicitly. I appreciate the effort to have it done automatically, but I found the underlying machinery a little bit confusing. I'm not entirely convinced that it is necessary, either. Couldn't each cog simply provide a namespace explicitly? Wo...

kwzrd

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

I probably would have named this to_dict. It feels like the name, copy, and the dosctring, Convert to dict and return, tell a different story.

kwzrd

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

This, and a few other methods below it, do not annotate return types. Is this intentional? Our linter is configured to ignore these as they are special methods, but I think in this case it really does make sense to have the annotations.

Here are the culprits: __setitem__, __getitem__, __delitem__, __contains__, __iter__, __len__

kwzrd

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Is this implemented because the ABC requires it? I'd probably delegate to __eq__ and negate it, but this is ok too. Just a little more code to read & maintain.

SebastiaanZ

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

SebastiaanZ

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Yes, it's a mixin method provided by the base class.

[regal arch]

jos-b

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

First quick review on phone, notably just missing password

jos-b

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

This won't work in production without a redis password. I have put the actual variable name in the org issue iirc but from memory it is REDIS_PASSWORD.

[regal arch]

lemonsaurus

[bot:redis\_persistence] 2 new commits

adf50a6 Changes discord-py to discord.py in Pipfile - lemonsaurus
ecf7f24 Add the REDIS_PASSWORD environment variable - lemonsaurus

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Yes. A normal dict has both a __ne__ and an __eq__, and so does the MutableMapping. Sadly, these don't seem to work with our object, so we must overload them or they'll cause trouble for us.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Good catch. I don't see any particular reason why we shouldn't. I think initially I didn't do it because I hadn't come up with the idea of making a JSONSerializableType annotation yet, but now that we've got that, there are obvious candidates for all of these to have return types. I will fix it.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

right, maybe, but this is just an overloaded standard dict method. we're replicating the functionality from dict.copy(), which would return a shallow copy of the dict object. We cannot really return a "copy" of the RedisDict, so we do the next best thing and just return a dict.

to_dict is probably a better name, but there's no dict.to_dict() - I mean, that would be silly.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

oh. yeah it is. good eye! will fix.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

I think this is a fair point, I don't mind making this change.

Could this lead to potential problems, if a cog is renamed or moved?

It's very unlikely, but this is why we should never use Redis to store anything important, only for caching data we can afford to lose. With that principle firmly in place, I don't really see the need to dive too deep into those kinds of edge cases.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Fixed in ecf7f24

MarkKoz

[python-discord/bot] Pull request review submitted: #860 discord\.py 1\.3\.x Functionality Migrating

Looks good, thanks! I cannot think of any other new 1.3 changes we can use. In any case, if there are some we missed, another PR can be opened down the road.

MarkKoz

[python-discord/bot] Pull request review submitted: #937 Token remover: catch ValueError when non\-ASCII chars are present

MarkKoz

[python-discord/bot] New review comment on pull request #937: Token remover: catch ValueError when non\-ASCII chars are present

No worries. I hope you don't get discouraged; your reviews would be valuable here.

kwzrd

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

kwzrd

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

this is just an overloaded standard dict method

Right, I didn't realize that. That does justify it a decent amount.

MarkKoz

[python-discord/bot] Pull request review submitted: #901 PEP command improvisations

MarkKoz

[python-discord/bot] New review comment on pull request #901: PEP command improvisations

It's confusing to follow the control flow of this loop. This could be simplified without a loop:

if pep_nr not in self.peps and (self.last_refreshed_peps + timedelta(minutes=30)) <= datetime.now():
    await self.refresh_peps_urls()

if pep_nr not in self.peps:
    ...
    await ctx.send(embed=embed)
    ...

[regal arch]

kwzrd

[python-discord/seasonalbot] Issue opened: #409 Specify event loop on Windows

Sister issue from bot repo: https://github.com/python-discord/bot/issues/894

With Python 3.8, the default asyncio event loop on Windows is now the ProactorEventLoop. One of our dependencies, aiodns, doesn't like that, as reported today by one of our contributors:

For bot, we've already...

[regal arch]

sco1

[python-discord/seasonalbot] New comment on pull request #408: Use input hashing instead of randomness inside 8ball

so a user cannot brute-force the 8ball until they obtain the answer they want

This is pretty much how I and everyone I know has ever used an actual magic 8ball.

Numerlor

[python-discord/seasonalbot] Pull request opened: #410 Set ayncio event loop to Selector on windows\.

Relevant Issues

Fixes #409

Description

Sets the event loop to Selector to prevent bot crashes on windows when an aiohttp request is made, as described in #409
Same implementation as in https://github.com/python-discord/bot/pull/903

[ebon magnet]

Build 20200517.2 succeeded

Requested by

GitHub

Duration

00:00:54

Build pipeline

Seasonal Bot

[regal arch]

MarkKoz

[python-discord/seasonalbot] Pull request review submitted: #410 Set ayncio event loop to Selector on windows\.

Tested locally on Windows. .branding refresh fails on master but works on your branch. Thanks for the quick fix.

Den4200

[python-discord/seasonalbot] Pull request review submitted: #410 Set ayncio event loop to Selector on windows\.

I've also tested locally on Windows, doing similarly as @MarkKoz. Works perfectly fine on this branch!

Den4200

[python-discord/seasonalbot] Issue closed: #409 Specify event loop on Windows

Den4200

[python-discord/seasonalbot] Pull request closed: #410 Set ayncio event loop to Selector on windows\.

Den4200

[seasonalbot:master] 2 new commits

540f98c Set ayncio event loop to Selector on windows. - Numerlor
f837344 Merge pull request #410 from Numerlor/windows-e... - Den4200

kwzrd

[python-discord/seasonalbot] New comment on pull request #410: Set ayncio event loop to Selector on windows\.

Thanks for the effort everyone!

[north knot]

Auto-deployed: seasonalbot

[ebon magnet]

Build 20200517.3 succeeded

Requested by

GitHub

Duration

00:02:21

Build pipeline

Seasonal Bot

[night lily]

SeasonalBot

Connected!

[regal arch]

Numerlor

[python-discord/seasonalbot] Issue opened: #411 Files without an encoding specified may crash bot

On windows (possibly some other OSs), non utf8 default encodings are used which produces UnicodeDecodeErrors when files with characters not included in the default encodings are read.

The main issue comes from bot\resources\evergreen\trivia_quiz.json which is always ran on bot startup, and crashes with the latin1 encoding (english windows); other files incompatible with the encoding are opened with an utf8 specified.

Here's a small list of where the encoding should be specified, but...

[ebon magnet]

Build 20200518.1 succeeded

Requested by

GitHub

Duration

00:01:20

Build pipeline

Bot

[regal arch]

Den4200

[seasonalbot:prefix\-rich\-presence] 3 new commits

540f98c Set ayncio event loop to Selector on windows. - Numerlor
f837344 Merge pull request #410 from Numerlor/windows-e... - Den4200
96230b0 Merge branch 'master' into prefix-rich-presence - Den4200

[ebon magnet]

Build 20200518.1 succeeded

Requested by

GitHub

Duration

00:01:01

Build pipeline

Seasonal Bot

[regal arch]

Den4200

[python-discord/seasonalbot] Pull request review submitted: #407 Show bot prefix in rich presence

Looks good!

Den4200

[python-discord/seasonalbot] Issue closed: #404 Prefix rich presence

Den4200

[python-discord/seasonalbot] Pull request closed: #407 Show bot prefix in rich presence

Den4200

[seasonalbot:master] 3 new commits

d84c949 Show bot prefix in rich presence - kwzrd
96230b0 Merge branch 'master' into prefix-rich-presence - Den4200
c919c5f Merge pull request #407 from python-discord/pre... - Den4200

Den4200

[python-discord/seasonalbot] branch deleted: prefix\-rich\-presence

[north knot]

Auto-deployed: seasonalbot

[ebon magnet]

Build 20200518.2 succeeded

Requested by

GitHub

Duration

00:02:05

Build pipeline

Seasonal Bot

[night lily]

SeasonalBot

Connected!

[ebon magnet]

Build 20200518.3 succeeded

Requested by

GitHub

Duration

00:00:54

Build pipeline

Seasonal Bot

[regal arch]

Den4200

[python-discord/seasonalbot] Pull request review submitted: #406 Fixed Prefix and Website

Looks great!

Den4200

[python-discord/seasonalbot] Issue closed: #374 \.snakes about has the wrong prefix

Den4200

[python-discord/seasonalbot] Pull request closed: #406 Fixed Prefix and Website

Den4200

[seasonalbot:master] 5 new commits

f280534 Updated Prefix and Website - DuckMasterAl
4ca9e78 Lint limit - DuckMasterAl
f1047af F i x e d S p a c e - DuckMasterAl
7051fef Merge branch 'master' into patch-1 - Den4200
dd8dd71 Merge pull request #406 from DuckMasterAl/patch-1 - Den4200

[north knot]

Auto-deployed: seasonalbot

[ebon magnet]

Build 20200518.4 succeeded

Requested by

GitHub

Duration

00:02:00

Build pipeline

Seasonal Bot

[night lily]

SeasonalBot

Connected!

[north knot]

Postgres backup completed!

[native jolt]

dependabot-preview[bot]

[flake8-annotations:dependabot/pip/flake8\-3\.8\.1] 1 new commit

403b789 Bump flake8 from 3.7.9 to 3.8.1 - dependabot-preview[bot]

dependabot-preview[bot]

[python-discord/flake8-annotations] New branch created: dependabot/pip/flake8\-3\.8\.1

dependabot-preview[bot]

[python-discord/flake8-annotations] Pull request opened: #84 Bump flake8 from 3\.7\.9 to 3\.8\.1

Bumps flake8 from 3.7.9 to 3.8.1.

Commits

f94e009 Release 3.8.1
00985a6 Merge branch 'issue638-ouput-file' into 'master'
e6d8a90 options: Forward --output-file to be reparsed for BaseFormatter
b4d2850 Release 3.8.0
03c7dd3 Merge branch 'exclude_dotfiles' into 'master'
9e67511 Fix using --exclude=.* to not match . and ..
6c4b5c8 Merge branch 'linters_py3' into 'master'
309db63 switch dogfood to use python3
8905a7a Merge branch 'logical_position_out_of_bound...

[regal arch]

kwzrd

[python-discord/seasonalbot] New comment on issue #411: Files without an encoding specified may crash bot

[flake8-file-encoding] does not check Pathlib's opens so I don't think it's worth adding to the linting process

Yeah, agreed. Thanks for looking into this.

This would be a good first issue if anyone is interested. If not, then I'll do it at some point.

[native jolt]

sco1

[python-discord/flake8-annotations] New comment on pull request #84: Bump flake8 from 3\.7\.9 to 3\.8\.1

Will be bumped by #83

sco1

[python-discord/flake8-annotations] Pull request closed: #84 Bump flake8 from 3\.7\.9 to 3\.8\.1

dependabot-preview[bot]

[python-discord/flake8-annotations] New comment on pull request #84: Bump flake8 from 3\.7\.9 to 3\.8\.1

OK, I won't notify you again about this release, but will get in touch when a new version is available.

If you change your mind, just re-open this PR and I'll resolve any conflicts on it.

dependabot-preview[bot]

[python-discord/flake8-annotations] branch deleted: dependabot/pip/flake8\-3\.8\.1

sco1

[python-discord/flake8-annotations] Issue closed: #82 Clarify how to fix ANN101 and ANN102

askoretskiy

[python-discord/flake8-annotations] New comment on issue #82: Clarify how to fix ANN101 and ANN102

I think the frustration comes from this part not clear from documentation. Maybe documentation should be improved (but I have no idea how :-)

[native jolt]

sco1

[python-discord/flake8-annotations] New comment on issue #82: Clarify how to fix ANN101 and ANN102

There are multiple methods for accomplishing this and you've provided two valid ones, via PEP 484 and PEP 563, in your original post.

The problems seem to be:

1. PyCharm doesn't resolve a bound TypeVar
2. The annotations seem redundant

We have no control or influence over the former, and the latter is a decision for you to make.

As discussed in #75:

The purpose of this package is not to enforce the related PEPs, as they make no mandates regarding full annotation coverage...

[regal arch]

SebastiaanZ

[python-discord/seasonalbot] New comment on pull request #408: Use input hashing instead of randomness inside 8ball

I think the randomness and the chance of getting a different answer for the same question is part of the 8ball experience. It's just a random answer generator, which is often implemented purely in hardware form using a container of fluid and a floating die. Brute-forcing your answer has been a feature since its introduction in 1950.

As such, I don't think we have a desire to make ours a non-standard implementation that deterministically tries to divine the Truth.

SebastiaanZ

[python-discord/seasonalbot] Pull request closed: #408 Use input hashing instead of randomness inside 8ball

[regal arch]

MarkKoz

[python-discord/bot] Pull request review submitted: #944 Increase snekbox re eval timeout to 30 seconds

I think 30 seconds is good enough.

[regal arch]

jos-b

[python-discord/bot] Pull request review submitted: #945 Add more statistics

Looks good, just a few minor changes to try make things clearer from a Grafana standpoint.

jos-b

[python-discord/bot] New review comment on pull request #945: Add more statistics

This should not be the same namespace as the stat for tracking eval failures/successes. Maybe:

            self.bot.stats.incr("snekbox_usages.roles.helpers")
        else:
            self.bot.stats.incr("snekbox_usages.roles.developers")

jos-b

[python-discord/bot] New review comment on pull request #945: Add more statistics

With the upcoming UNIX command snekbox system this could use better naming, maybe:

                self.bot.stats.incr("snekbox.python.fail")
            elif icon in (":warning:", ":white_check_mark:"):
                self.bot.stats.incr("snekbox.python.success")

jos-b

[python-discord/bot] New review comment on pull request #945: Add more statistics

Same issue as above:

            self.bot.stats.incr("snekbox_usages.channels.help")
        elif ctx.channel.id == Channels.bot_commands:
            self.bot.stats.incr("snekbox_usages.channels.bot_commands")
        else:
            self.bot.stats.incr("snekbox_usages.channels.topical")

SCHTAMP

[python-discord/bot] Fork created: SCHTAMP/bot

jos-b

[python-discord/bot] New comment on pull request #945: Add more statistics

Relevant organisation issue: python-discord/organisation#232

[ebon magnet]

Build 20200518.2 succeeded

Requested by

GitHub

Duration

00:01:23

Build pipeline

Bot

[regal arch]

Den4200

[python-discord/bot] Pull request review submitted: #941 Add message publishing to Reddit cog

Looks good.

Den4200

[python-discord/bot] Issue closed: #940 Publish messages to the \#reddit channel

Den4200

[python-discord/bot] Pull request closed: #941 Add message publishing to Reddit cog

Den4200

[bot:master] 3 new commits

d7e6bed Add message publishing to Reddit cog - ks129
dac35cc Merge branch 'master' into reddit-publish - Den4200
dd22088 Merge pull request #941 from ks129/reddit-publish - Den4200

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200518.3 succeeded

Requested by

GitHub

Duration

00:02:53

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

jos-b

[bot:master] 1 new commit

bd4b439 [bug] Adjustment to changes in #941, return mes... - jos-b

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200519.1 succeeded

Requested by

Joseph Banks

Duration

00:02:53

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

sco1

[bot:master] 1 new commit

1d36ec7 Add Steam gift card scam to domain blacklist - sco1

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200519.2 succeeded

Requested by

GitHub

Duration

00:02:47

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

jodth07

[python-discord/seasonalbot] New comment on issue #411: Files without an encoding specified may crash bot

Working on it

[regal arch]

decorator-factory

[python-discord/bot] Pull request opened: #952 Commands for convenient regex testing

• create RegularExpressions cog
• create !regexp search command

[ebon magnet]

Build 20200519.3 failed

Requested by

GitHub

Duration

00:01:05

Build pipeline

Bot

[regal arch]

decorator-factory

[python-discord/bot] Issue opened: #953 Create commands related to regular expressions

Rationale

Sometimes people ask regex-related questions. In order to test a particular regular expression or to demonstrate that it works, helpers have to do this:

import re
print(re.search(r"hello\s*world"))

There are a few issues with this approach:

• non-helpers simply can't to that
• this is inconvenient -- it's quite a setup
• this doesn't highlight the matched groups

To solve these issues, I propose to create a !regexp command that would handle that.

Usag...

[ebon magnet]

Build 20200519.4 succeeded

Requested by

GitHub

Duration

00:01:16

Build pipeline

Bot

[regal arch]

Den4200

[python-discord/seasonalbot] Pull request review submitted: #403 Handle staff users and DMs in the halloweenify cog

Looks all good to me!

Den4200

[python-discord/seasonalbot] Issue closed: #402 Handle potential errors in halloweenify command

Den4200

[python-discord/seasonalbot] Pull request closed: #403 Handle staff users and DMs in the halloweenify cog

Den4200

[seasonalbot:master] 4 new commits

ff6ad8d Handle staff users and DMs in the halloweenify cog - Akarys42
3f7a0a2 Update halloweenify error message to make it mo... - Akarys42
9f0e029 Merge branch 'master' into boo-fix - Den4200
df75d18 Merge pull request #403 from Akarys42/boo-fix - Den4200

[north knot]

Auto-deployed: seasonalbot

[night lily]

SeasonalBot

Connected!

[regal arch]

Den4200

[python-discord/bot] Pull request review submitted: #944 Increase snekbox re eval timeout to 30 seconds

30 seconds should be fine.

[ebon magnet]

Build 20200519.5 succeeded

Requested by

GitHub

Duration

00:01:20

Build pipeline

Bot

[regal arch]

Den4200

[python-discord/bot] Pull request closed: #944 Increase snekbox re eval timeout to 30 seconds

Den4200

[bot:master] 5 new commits

d2c538e Increase snekbox re eval timeout. - Numerlor
1c06d2a Move the re eval timeout to a module constant - Numerlor
5a48ed0 Change tests to use the new timeout constant - Numerlor
73943c2 Merge branch 'master' into eval-timeout-increase - Den4200
9c7627a Merge pull request #944 from Numerlor/eval-time... - Den4200

[north knot]

Auto-deployed: bot

[oak estuary]

Python Bot

Connected!

[regal arch]

ks129

[python-discord/bot] New comment on pull request #945: Add more statistics

@jos-b Made fixes

[north knot]

Postgres backup completed!

[regal arch]

ks129

[python-discord/bot] Pull request opened: #954 Error Handler tests

Closes #575

About PR

• Created tests for all error handler functions
• Made 2 small changes in handler itself:
  • In 3ea33bc I changed hasattr to getattr to make testing possible.
  • In e950def made that this request coroutine only when this is required to avoid not awaited coroutines.

[ebon magnet]

Build 20200519.10 succeeded

Requested by

GitHub

Duration

00:01:53

Build pipeline

Bot

[regal arch]

ks129

[python-discord/seasonalbot] Issue opened: #412 Update help command

Description

python-discord/bot#519 got merged some days ago so these changes should get merged to here too.

Reasoning

Because current help command is basically Python bot help command, just some small changes.

Would you like to implement this yourself?

• [ ] I'd like to implement this feature myself
• [x] Anyone can implement this feature

[regal arch]

decorator-factory

[python-discord/bot] Issue closed: #953 Create \`\!regexp\` command

decorator-factory

[python-discord/bot] Pull request closed: #952 Commands for convenient regex testing

decorator-factory

[python-discord/bot] Issue reopened: #953 Create \`\!regexp\` command

decorator-factory

[python-discord/bot] Pull request reopened: #952 Commands for convenient regex testing

[ebon magnet]

Build 20200520.1 succeeded

Requested by

GitHub

Duration

00:01:37

Build pipeline

Bot

[regal arch]

TeamSpen210

[python-discord/bot] Pull request review submitted: #952 Commands for convenient regex testing

TeamSpen210

[python-discord/bot] New review comment on pull request #952: Commands for convenient regex testing

Minor typos here.

        """Commands for exploring the mysterious world of regular expressions."""

jos-b

[python-discord/bot] New comment on pull request #952: Commands for convenient regex testing

I'll take a more in depth look at the code soon, but one question I have now is whether executing arbitrary RegExp code is safe?

We used to have an issue on the bot where the RegExp used for code block detection resulted in the bot locking up due something called catastrophic backtracing. How can we prevent unsafe regular expressions from locking up the bot?

jos-b

[python-discord/bot] New comment on pull request #952: Commands for convenient regex testing

Found the conversation for when we discovered the issue in the bot, the solution was to adjust the regular expression to something safe but this still leaves the risk of abuse if people use dangerous regular expressions.

decorator-factory

[python-discord/bot] New comment on pull request #952: Commands for convenient regex testing

@jos-b Why not set a small time limit?

jos-b

[python-discord/bot] New comment on pull request #952: Commands for convenient regex testing

Right, but then we need to isolate the process which would require migrating a regex executor to another platform (which with the new UNIX commands in snekbox and possibility for more things in future is feasible) which would make this implementation obsolete.

TeamSpen210

[python-discord/bot] New comment on pull request #952: Commands for convenient regex testing

Could they use snekbox somehow? Format the regex into a template snippet which executes it, then prints to stdout a JSON result. Then parse that in the bot, handling errors.

jos-b

[python-discord/bot] New comment on pull request #952: Commands for convenient regex testing

Example of catastrophic backtracing from StackOverflow

jos-b

[python-discord/bot] New comment on pull request #952: Commands for convenient regex testing

Snekbox could definitely be used, either by adding a new executor or by basically sending all the processing code as a regular eval to snekbox.

jos-b

[python-discord/bot] New comment on pull request #952: Commands for convenient regex testing

However at that point I think it becomes something where we need to discuss things among the core developers before we can think about implementation detail.

[regal arch]

decorator-factory

[python-discord/bot] New comment on pull request #952: Commands for convenient regex testing

The regex module seems to handle such regular expressions without any issues. All examples of catastrophic regexes I could find work instantly.

jos-b

[python-discord/bot] New comment on pull request #952: Commands for convenient regex testing

The example from the StackOverflow link I posted above results in 100% CPU when tested locally:
<img width="557" alt="image" src="https://user-images.githubusercontent.com/20439493/82393339-f7d78280-9a3d-11ea-82a4-ee506e33fa74.png">

decorator-factory

[python-discord/bot] New comment on pull request #952: Commands for convenient regex testing

I meant the third-party regex module:

jodth07

[python-discord/seasonalbot] Pull request opened: #413 Encoding bug fix

Relevant Issues

• Cleanup: Add explicit encoding on open() calls #334
• Files without an encoding specified may crash bot #411

Closes #334
Closes #411

Description

Added explicit encoding to utf8 for cogs that open json or other text files.

Reasoning

Though this did not cause errors in Unix based OS, it was causing errors when loading some of those files in windows OS.

Additional Details

We may require explicit encoding going forward for other cogs being a...

[ebon magnet]

Build 20200520.1 succeeded

Requested by

GitHub

Duration

00:01:00

Build pipeline

Seasonal Bot

[regal arch]

decorator-factory

[python-discord/bot] New comment on pull request #952: Commands for convenient regex testing

A possible issue with regex is that it adds some extra regex features, so a regular expression might work with the command, but not in python with re, which might confuse people. A simple fix would be to parse the regex using re as well and see if it throws an error.

decorator-factory

[python-discord/bot] Pull request review submitted: #952 Commands for convenient regex testing

decorator-factory

[python-discord/bot] New review comment on pull request #952: Commands for convenient regex testing

Thanks, I'll fix that. I think format_* docstrings can be improved as well.

[north knot]

Postgres backup completed!

[regal arch]

ks129

[python-discord/bot] Pull request opened: #955 Source command

Closes #331

Created !source command, that supports 3 types of source items: Help command, command, cog. Send embed that has a link to GitHub, help text of the item and when command/cog, are you able to run it in this context.

[ebon magnet]

Build 20200520.2 succeeded

Requested by

GitHub

Duration

00:01:45

Build pipeline

Bot

[ebon magnet]

Build 20200520.3 succeeded

Requested by

GitHub

Duration

00:01:23

Build pipeline

Bot

[regal arch]

thomaspet

[python-discord/bot] Pull request review submitted: #864 Ban and kick reason length fix

Embed description length, rest looks good. Can't 100% verify tests as I'm not familiar with the setup yet

thomaspet

[python-discord/bot] New review comment on pull request #864: Ban and kick reason length fix

This will create a 2049 length string, if it's above 2048

[regal arch]

kwzrd

[python-discord/seasonalbot] New comment on pull request #413: Encoding bug fix

Thanks, I'll try to review this soon.

[regal arch]

lemonsaurus

[python-discord/bot] New review comment on pull request #864: Ban and kick reason length fix

Is there supposed to be a space before the ... here? Doesn't seem like you're doing that anywhere else. If so, why?

lemonsaurus

[python-discord/bot] Pull request review submitted: #864 Ban and kick reason length fix

Need a couple of answers here before I can approve but it's starting to look pretty good to me.

lemonsaurus

[python-discord/bot] New review comment on pull request #864: Ban and kick reason length fix

Can you be more specific? What warning does this raise, and how does this avoid it? This code seems weird to me.

lemonsaurus

[python-discord/bot] New review comment on pull request #864: Ban and kick reason length fix

Yeah, I'm a little suspicious of these hardcoded values too. Can you justify them? Is there a test for this particular part of the code? Have you tested this manually?

[regal arch]

ks129

[python-discord/bot] Pull request review submitted: #864 Ban and kick reason length fix

ks129

[python-discord/bot] New review comment on pull request #864: Ban and kick reason length fix

I will fix this length issue, but I can't change this. Otherwise, we are getting this issue. This already tests it here, but I have same issue there too.

[ebon magnet]

Build 20200520.4 failed

Requested by

GitHub

Duration

00:00:57

Build pipeline

Bot

Build 20200520.5 failed

Requested by

GitHub

Duration

00:01:02

Build pipeline

Bot

[regal arch]

lemonsaurus

[python-discord/bot] Pull request review submitted: #864 Ban and kick reason length fix

lemonsaurus

[python-discord/bot] New review comment on pull request #864: Ban and kick reason length fix

That's fine, just make sure the values are correct. It's mostly this length issue I was concerned about.

[ebon magnet]

Build 20200520.7 failed

Requested by

GitHub

Duration

00:01:02

Build pipeline

Bot

[regal arch]

jos-b

[python-discord/bot] Pull request review submitted: #945 Add more statistics

This looks good to me!

[ebon magnet]

Build 20200520.8 failed

Requested by

GitHub

Duration

00:01:05

Build pipeline

Bot

Build 20200520.10 failed

Requested by

GitHub

Duration

00:01:07

Build pipeline

Bot

Build 20200520.11 failed

Requested by

GitHub

Duration

00:01:26

Build pipeline

Bot

[regal arch]

jos-b

[python-discord/seasonalbot] Pull request review submitted: #413 Encoding bug fix

[ebon magnet]

Build 20200520.12 failed

Requested by

GitHub

Duration

00:01:00

Build pipeline

Bot

Build 20200520.13 failed

Requested by

GitHub

Duration

00:01:04

Build pipeline

Bot

Build 20200520.14 succeeded

Requested by

GitHub

Duration

00:01:30

Build pipeline

Bot

[regal arch]

lemonsaurus

[python-discord/seasonalbot] Pull request review submitted: #413 Encoding bug fix

MarkKoz

[python-discord/bot] Pull request review dismissed: #864 Ban and kick reason length fix

Something has gone wrong with the formatting:

ikuyarihS

[python-discord/bot] Pull request review dismissed: #864 Ban and kick reason length fix

Looking good, one more nag and it'll be good to go.

lemonsaurus

[python-discord/bot] Pull request review submitted: #864 Ban and kick reason length fix

Okay. LGTM.

SebastiaanZ

[python-discord/bot] Pull request review submitted: #930 Add tests for the antimalware cog

SebastiaanZ

[python-discord/bot] New review comment on pull request #930: Add tests for the antimalware cog

Why are you monkeypatching this directly instead of using unittest.mock.patch? The downside of monkeypatching directly is that the monkeypatch will stay behind, as the object in memory is mutated. While unittest.mock.patch does the same, in principle, it restores the original after the fact. By providing an argument for new, we can determine what it should be patched with and it makes sure that the mock doesn't get passed as an attribute to test methods.

You could use `unittest.mock....

SebastiaanZ

[python-discord/bot] Pull request review submitted: #930 Add tests for the antimalware cog

There's currently a direct monkey patch in your setUp method of the first test case class. The downside is that monkeypatching directly mutates the object in memory and will leave the change behind after the tests in this file have finished. I've suggested an alternative using unittest.mock.patch in the specific comment for that line.

[regal arch]

Numerlor

[python-discord/bot] New comment on pull request #955: Source command

Before a proper review,
The embed when not found could use some text instead of just being empty like that.

And what's the reason for the two identical links in the title and the footer?

[ebon magnet]

Build 20200520.15 failed

Requested by

GitHub

Duration

00:00:26

Build pipeline

Bot

[ebon magnet]

Build 20200520.16 failed

Requested by

GitHub

Duration

00:01:15

Build pipeline

Bot

[ebon magnet]

Build 20200520.17 succeeded

Requested by

GitHub

Duration

00:02:25

Build pipeline

Bot

Build 20200520.18 succeeded

Requested by

GitHub

Duration

00:01:26

Build pipeline

Bot

[regal arch]

ks129

[python-discord/bot] New comment on pull request #955: Source command

@Numerlor Is this better now? I removed the link from the title and made converter raising BadArgument instead just sending bot repo when invalid argument provided.

[native jolt]

sco1

[python-discord/flake8-annotations] Issue opened: #85 Add Python 3\.9 Support

With Python 3.9 entering beta we're at a good point to begin to evaluate for any potential issues with the new version. This should be an uneventful update; local runs of the test suite with 3.9.0b1 presents no issues.

This is stalled pending CI support for Python 3.9. See: https://github.com/actions/virtual-environments/issues/912

Python 3.9 final release is [expected in ...

[native jolt]

sco1

[python-discord/flake8-annotations] Fork created: sco1/flake8-annotations

[regal arch]

jpark9013

[python-discord/bot] Fork created: jpark9013/bot

[north knot]

Postgres backup completed!

[regal arch]

ks129

[python-discord/bot] Pull request review submitted: #952 Commands for convenient regex testing

Found one bug. One other thing I want to say is when I tested it, all results (errors and matches) is so plain. I think some language formatting should be added to there. This will increase readability.
for errors I made this with diff formatting and added - before each line, then I got the red text. And maybe should success result inside embed that ...

ks129

[python-discord/bot] New review comment on pull request #952: Commands for convenient regex testing

This will not work. We migrated to new help command lately, so this has to be changed:

        await ctx.send_help(ctx.command)

MarkKoz

[python-discord/bot] Pull request review submitted: #942 Python News: Implement stats

[ebon magnet]

Build 20200521.1 succeeded

Requested by

GitHub

Duration

00:01:24

Build pipeline

Bot

[regal arch]

ks129

[python-discord/bot] Pull request opened: #956 Help System: Move dormant faster if help channel is empty

Closes #939

• Implemented on_message_delete, that reschedule a task, when is_empty return True and channel is inside help in-use category.
• move_idle_channel check, is channel empty and when it is, this uses correct (shorter) idle_seconds.
• is_empty request last message and then check, is this bot available message.

[ebon magnet]

Build 20200521.2 succeeded

Requested by

GitHub

Duration

00:01:16

Build pipeline

Bot

[regal arch]

Aaryan-K

[python-discord/seasonalbot] New comment on issue #174: \[Feature Request\] Earth Photos

This seems to be beyond my ability - if it can be unassigned I can look for an issue that's more my level of experience to contribute to.

kwzrd

[python-discord/bot] New comment on issue #926: RuntimeError: Line exceeds maximum page size 998

This bug also affects talentpool history. Triggered today:

Exception is raised on the same line, so this falls under the same Sentry issue and probably requires no extra attention since the on-going work should automatically resolve this as well.

[regal arch]

aeros

[python-discord/bot] New comment on issue #926: RuntimeError: Line exceeds maximum page size 998

Good to know @kwzrd, I'll have to remember to test the functionality of the talent pool history as well for larger messages.

Also, regarding the PR, I've been a bit occupied w/ some last-minute 3.9beta changes and will be away this weekend, but I should have time to wrap this up next week.

[regal arch]

ks129

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

Some small notes

ks129

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

I think this will be here better:

            namespace += "_"

ks129

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

What about adding first argument (default False) and when True, pop and get first item.

ks129

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Why value variable is required? Isn't better

        if self.get(key) is None:

SebastiaanZ

[python-discord/bot] New comment on pull request #947: RedisDict \- Data Persistence

Hello, this PR is stalled until further notice. It's going to get a proper rewrite.

Den4200

[python-discord/bot] Pull request review submitted: #935 ModLog: ignore DMs in the message delete listener

[regal arch]

MarkKoz

[python-discord/bot] New review comment on pull request #956: Help System: Move dormant faster if help channel is empty

The documentation should be clearer.

        """
        Reschedule an in-use channel to become dormant sooner if the channel is empty.

        The new time for the dormant task is configured with `HelpChannels.deleted_idle_minutes`. 
        """

MarkKoz

[python-discord/bot] Pull request review submitted: #956 Help System: Move dormant faster if help channel is empty

MarkKoz

[python-discord/bot] New review comment on pull request #956: Help System: Move dormant faster if help channel is empty

Take a look at is_dormant_message. It strips the messages and also checks that the author is this bot and not some other bot, but basically does the same thing as is_empty. You could make is_dormant_message a generic function that can compare to any message and then call that in is_empty.

MarkKoz

[python-discord/bot] New review comment on pull request #956: Help System: Move dormant faster if help channel is empty

        """Return True if the most recent message in `channel` is the bot's `AVAILABLE_MSG`."""

MarkKoz

[python-discord/bot] New review comment on pull request #945: Add more statistics

Safe to simplify this to an else.

            else:

MarkKoz

[python-discord/bot] Pull request review submitted: #945 Add more statistics

MarkKoz

[python-discord/bot] New review comment on pull request #945: Add more statistics

We don't allow this command in DMs, right? Cause if channel is a DMChannel, then category_id will be an AttributeError.

I want to note I have a PR which adds a utility function for checking help channels. It's not done yet, but when it is, this should be replaced with it. https://github.com/python-discord/bot/blob/4378e1f12907a73a26e56760c4a5717ff2a017d2/bot/utils/channel.py#L10

MarkKoz

[python-discord/bot] New review comment on pull request #950: Created tests for startup logging

If you retrieved the embed from the call args, then you already know it's been called with that value. It doesn't make sense to use a value from its call args in such an assertion. In such case, I would advice you just use assert_awaited_once().

MarkKoz

[python-discord/bot] Pull request review submitted: #950 Created tests for startup logging

MarkKoz

[python-discord/bot] New review comment on pull request #950: Created tests for startup logging

This is something I've brought up a few times in various PRs. Basically, I don't think we should assert exact values of strings. In most cases it doesn't really matter what the exact value is. What the test should really care about is if it uses the value we wanted to. For example, rather than testing that it shows "x" every time, we should test that if we give it "x", it will show "x", but if we gave it "y", it'd show "y". This can be achieved by defining and using constants rather than usin...

[regal arch]

lemonsaurus

[bot:redis\_persistence] 3 new commits

cc3591d Add the REDIS_PASSWORD environment variable - lemonsaurus
23c2e7a Replace redis-py with aioredis. - lemonsaurus
3f596d5 Opens a Redis connection in the Bot class. - lemonsaurus

[regal arch]

MarkKoz

[bot:bug/filters/928/non\-ascii\-token] 2 new commits

e23aa88 Token remover: decode ID using URL-safe base64 - MarkKoz
95ef2dc Token remover: match only base64 in regex - MarkKoz

[ebon magnet]

Build 20200522.1 failed

Requested by

GitHub

Duration

00:01:01

Build pipeline

Bot

[ebon magnet]

Build 20200522.2 failed

Requested by

GitHub

Duration

00:00:55

Build pipeline

Bot

[ebon magnet]

Build 20200522.3 succeeded

Requested by

GitHub

Duration

00:01:18

Build pipeline

Bot

Build 20200522.4 succeeded

Requested by

GitHub

Duration

00:01:07

Build pipeline

Bot

Build 20200522.5 succeeded

Requested by

GitHub

Duration

00:01:19

Build pipeline

Bot

[regal arch]

ks129

[python-discord/bot] Pull request review submitted: #945 Add more statistics

ks129

[python-discord/bot] New review comment on pull request #945: Add more statistics

Yes, this have guild only decorator.

[ebon magnet]

Build 20200522.6 succeeded

Requested by

GitHub

Duration

00:01:20

Build pipeline

Bot

[north knot]

Postgres backup completed!

[regal arch]

jos-b

[python-discord/bot] Pull request closed: #942 Python News: Implement stats

jos-b

[bot:master] 4 new commits

206aed7 Python News: Implement stats - ks129
7d9dc3a Merge branch 'master' into python-news-stats - ks129
5b1fc5f Merge branch 'master' into python-news-stats - MarkKoz
8d2e1df Merge pull request #942 from ks129/python-news-... - jos-b

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200522.7 succeeded

Requested by

Joseph Banks

Duration

00:03:00

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

SebastiaanZ

[python-discord/bot] New review comment on pull request #836: Tags cog unit tests and tags cog fixes

This like is a bit problematic, as it assumes the prefix to be !. This may work for production, but a lot of testing bots are set up to use a different prefix in their config.yml. The consequence of that is that running tests locally, like people are hopefully doing after they've made changes, will show a failing test

AssertionError: 'To show a tag, type +tags <tagname>.' != 'To show a tag, type !tags <tagname>.'
- To show a tag, type +tags <tagname>.
?                     ^
...

SebastiaanZ

[python-discord/bot] Pull request review submitted: #836 Tags cog unit tests and tags cog fixes

SebastiaanZ

[python-discord/bot] New review comment on pull request #836: Tags cog unit tests and tags cog fixes

Not a required change, more of a discussion.

I would have opted for an approach that provided the function with test data instead of executing the actual function on the actual data. That way, it's easy to assert not only if the keys that we set are present in the dictionary, but also that the actual parsing of the tag file and Path have worked correctly. That's the critical part of our tags storage system, I think (being able to parse the format in which we store tags), while I'm fairly c...

[ebon magnet]

Build 20200522.8 succeeded

Requested by

GitHub

Duration

00:02:41

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[ebon magnet]

Build 20200522.9 succeeded

Requested by

GitHub

Duration

00:02:07

Build pipeline

Bot

[regal arch]

MarkKoz

[python-discord/bot] Pull request review submitted: #956 Help System: Move dormant faster if help channel is empty

Fixed missing awaits and also did some refactoring. Tested it locally and it works.

MarkKoz

[python-discord/bot] Pull request review submitted: #950 Created tests for startup logging

[regal arch]

MarkKoz

[python-discord/bot] Pull request review submitted: #945 Add more statistics

[ebon magnet]

Build 20200522.10 succeeded

Requested by

GitHub

Duration

00:02:06

Build pipeline

Bot

[regal arch]

jos-b

[python-discord/bot] Pull request closed: #945 Add more statistics

jos-b

[bot:master] 10 new commits

93cce50 Stats: Create guild boost stat collection - ks129
1a6abaa Stats: Added codeblock correction stats - ks129
158e19a Stats: Added stats for eval successes + fails - ks129
5878ec9 Stats: Added stats for eval role uses (Helpers/... - ks129
613b00a Stats: Added stats for eval channel using (Help... - ks129

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200522.11 succeeded

Requested by

Joseph Banks

Duration

00:04:17

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

lemonsaurus

[bot:redis\_persistence] 1 new commit

57fe4bf Set up async testbed - lemonsaurus

[regal arch]

MarkKoz

[bot:redis\_persistence] 5 new commits

fd6f3d3 Fix assertion for create_task in duck pond tests - MarkKoz
45e6f8d Improve aiohttp context manager mocking in snek... - MarkKoz
6aed2f6 Fix unawaited coro warning when instantiating B... - MarkKoz
1ad7833 Properly mock the redis pool in MockBot - MarkKoz
d8f1634 Use autospecced mocks in MockBot for the stats ... - MarkKoz

[regal arch]

lemonsaurus

[bot:redis\_persistence] 1 new commit

eb63fb0 Finish .set and .get, and add tests. - lemonsaurus

[ebon magnet]

Build 20200523.1 succeeded

Requested by

GitHub

Duration

00:01:11

Build pipeline

Seasonal Bot

[regal arch]

Den4200

[python-discord/seasonalbot] Pull request review submitted: #401 handle disabled DMs when starting Minesweeper

Everything looks good!

Den4200

[python-discord/seasonalbot] Issue closed: #399 Handle disabled DMs when starting Minesweeper

Den4200

[python-discord/seasonalbot] Pull request closed: #401 handle disabled DMs when starting Minesweeper

Den4200

[seasonalbot:master] 5 new commits

2a10ffb handle disabled DMs when starting Minesweeper - fuzzmz
c3fae28 update minesweeper dm error message - fuzzmz
0aed8ab exit minesweeper early if DM disabled - fuzzmz
29304c0 Merge branch 'master' into minesweeper-disabled... - Den4200
f43c3f0 Merge pull request #401 from fuzzmz/minesweeper... - Den4200

[north knot]

Auto-deployed: seasonalbot

[ebon magnet]

Build 20200523.2 succeeded

Requested by

GitHub

Duration

00:02:15

Build pipeline

Seasonal Bot

[night lily]

SeasonalBot

Connected!

[north knot]

Postgres backup completed!

[regal arch]

ks129

[python-discord/bot] New comment on issue #582: Write unit tests for \`bot/cogs/jams\.py\`

I'm gonna work with this issue.

[regal arch]

lemonsaurus

[bot:redis\_persistence] 2 new commits

c8a9a77 Finish asyncifying RedisCache methods - lemonsaurus
387bf5c Complete asyncified test suite for RedisCache - lemonsaurus

lemonsaurus

[bot:redis\_persistence] 1 new commit

5bd8e13 Better docstring for RedisCache - lemonsaurus

[regal arch]

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

With the rewrite, this feels less relevant. _namespace is now really only ever set in this method, and is only initialized as None in __init__. This is done because __init__ happens too early in the lifecycle for us to be able to set the attribute yet. We need to wait until __set_name__.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Done.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

This method has been removed.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

No longer relevant.

lemonsaurus

[python-discord/bot] New comment on pull request #947: RedisDict \- Data Persistence

This PR is now once again ready for review and for testing.

I've written a bunch of tests for it which all pass, but I haven't given it a proper test in a test bot yet, and this should absolutely be done before we merge it. It'll be a nice learning case for any reviewer to get to implement this into a testcog anyway, so I think I'll let you guys handle the bot testing.

jos-b

[python-discord/bot] Pull request review dismissed: #947 RedisDict \- Data Persistence

First quick review on phone, notably just missing password

lemonsaurus

[bot:redis\_persistence] 38 new commits

2559bc9 Add mutability.md tag - decorator-factory
028781a header->bold in mutability.md - decorator-factory
a946842 Fix hard-wrapping in mutability.md - decorator-factory
d7e6bed Add message publishing to Reddit cog - ks129
206aed7 Python News: Implement stats - ks129

[ebon magnet]

Build 20200523.1 failed

Requested by

GitHub

Duration

00:01:39

Build pipeline

Bot

[regal arch]

lemonsaurus

[bot:redis\_persistence] 3 new commits

db75440 Better docstring for RedisCache.contains - lemonsaurus
54bb228 Merge branch 'redis_persistence' of github.com:... - lemonsaurus
fc1999e Unbreak the error_handler - lemonsaurus

[ebon magnet]

Build 20200523.2 failed

Requested by

GitHub

Duration

00:02:31

Build pipeline

Bot

[regal arch]

SebastiaanZ

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Thanks.

SebastiaanZ

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

Very nice work. I've only done a code review as I don't have a fully functional dev set-up on the laptop I'm currently working on.

SebastiaanZ

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Nice method to solve this issue.

SebastiaanZ

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

The reason that this worked before is that when you "call" a Mock, it returns the same child mock each time. That means the lack of an argument to post() does not matter here: All you care about is getting the child mock that you want to manipulate the __aenter__ return value of.

self.bot.http_session.post() will return the mock that will be returned later when the actual code we want to test is run and the return value of that mocks __aenter__ mock is configured.

SebastiaanZ

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

This docstring mentions the .copy method, but it's testing the .to_dict method.

SebastiaanZ

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Other "lazy" session creators hook into the _recreate method below to do their work. What's the reason for doing it here instead of there? I don't think I mind either way, this will ensure that it will be scheduled as soon as the loop starts.

The only thing I'm slightly worried about is that I don't know when Discord calls Bot.close(): If that's done before it calls Bot.clear(), your redis session will be closed but not recreated.

Edit: A quick glance at discord.py makes me thin...

[regal arch]

lemonsaurus

[bot:redis\_persistence] 2 new commits

49492ff Moving the Redis session creation to Bot._recreate - lemonsaurus
489f940 CI needs REDIS_PASSWORD to pass tests - lemonsaurus

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

I've moved it over to _recreate anyway. I didn't do it originally because I just wasn't aware of it. Seems like a better fit for that method, as long as it works.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

I think this was written by @MarkKoz, I don't recognize it.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

This should also be @MarkKoz's work. Nice work, Mark.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

hmm.. I can't imagine what an empty float or int would look like. I don't think that's possible to create, no.

[ebon magnet]

Build 20200523.3 succeeded

Requested by

GitHub

Duration

00:03:02

Build pipeline

Bot

[regal arch]

lemonsaurus

[bot:redis\_persistence] 1 new commit

1ea4718 Update exception message

• lemonsaurus

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Yeah, I saw the same docs, so I figure it's fine.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Yeah I really love this syntax for asynciterators that @jos-b found, it's very elegant.

[ebon magnet]

Build 20200523.4 succeeded

Requested by

GitHub

Duration

00:02:16

Build pipeline

Bot

[regal arch]

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Oops. I'll fix that. :eye:

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Yep, I guess we're testing that already.

SebastiaanZ

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

SebastiaanZ

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

I don't know either and I can't think of a situation where we would set such an empty int/float string with our serialization method. I came across this before I read the _to_typestring.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

I've added some documentation for this, but I also decided to not rely on it for .pop, since it's just an additional superfluous API call for no real benefit.

kwzrd

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

kwzrd

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

I wonder if these could be defined in one place?

A very simple example:

TEMPLATES = (
    (float, "f|{value}"),
    (int,   "i|{value}"),
    (str,   "s|{value}"),
)

Both functions then use this definition, e.g. by lopping through the pairs until it finds a match. I don't love that this is essentially a bi-directional map with each direction defined elsewhere, seems fragile. Both functions also return None should no match be found, which is not annotated nor documen...

lemonsaurus

[bot:redis\_persistence] 2 new commits

aa0bb02 Fix typo in test_to_dict docstring - lemonsaurus
2fb8625 Don't rely on HDEL ignoring keys for .pop - lemonsaurus

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Done!

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

An interesting thought. I'll make an attempt at this.

kwzrd

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

kwzrd

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Perhaps better to define the prefix, not the template, e.g.:

(float, "f|")

Makes it easier for the startswith check.

[ebon magnet]

Build 20200523.5 succeeded

Requested by

GitHub

Duration

00:02:15

Build pipeline

Bot

[regal arch]

Numerlor

[python-discord/seasonalbot] Pull request review submitted: #413 Encoding bug fix

the file opens like in here should also take the utf8 enbcoding or can fail like the reads do https://github.com/python-discord/seasonalbot/blob/f43c3f014fc2e2d690e572ff2a5d1b79b33836df/bot/exts/halloween/hacktoberstats.py#L148

lemonsaurus

[bot:redis\_persistence] 1 new commit

5120717 DRY approach to typestring prefix resolution - lemonsaurus

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Yep, that's what I went for. It's addressed in 5120717

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

I'll resolve this then.

[ebon magnet]

Build 20200523.6 succeeded

Requested by

GitHub

Duration

00:01:57

Build pipeline

Bot

[regal arch]

ks129

[python-discord/bot] Pull request opened: #957 Code Jams unit tests

Created tests for CodeJam cog + it's setup function.

Closes #582

[ebon magnet]

Build 20200523.7 succeeded

Requested by

GitHub

Duration

00:01:23

Build pipeline

Bot

[regal arch]

kwzrd

[python-discord/bot] Pull request review submitted: #947 RedisDict \- Data Persistence

kwzrd

[python-discord/bot] New review comment on pull request #947: RedisDict \- Data Persistence

Solid work - thanks for accepting the suggestion. I think this ends up being a lot safer and convenient, as the TYPESTRING_PREFIXES definition now also serves as documentation.

Den4200

[python-discord/bot] Pull request review submitted: #936 Filtering: don't attempt to send additional embeds for invalid invites

Tested locally and it works!

Den4200

[python-discord/bot] Pull request review submitted: #934 Filtering: don't delete messages in DMs

Looks fine.

Den4200

[bot:bug/filters/933/dont\-delete\-in\-dms] 108 new commits

05616ae Refactor the Help command. - mathsman5133
87f9fdd Minor formatting changes to align with current ... - mathsman5133
5ebb87d Add a special case for when the help command in... - mathsman5133
79d5b2d Few changes to keep formatting same as current - mathsman5133
d8384b2 Show a maximum of 8 commands per page rather th... - mathsman5133

[ebon magnet]

Build 20200523.8 succeeded

Requested by

GitHub

Duration

00:01:19

Build pipeline

Bot

[regal arch]

Den4200

[python-discord/bot] Issue closed: #933 Message Filters Should Not Attempt to Delete Messages in DMs

Den4200

[python-discord/bot] Pull request closed: #934 Filtering: don't delete messages in DMs

Den4200

[bot:master] 3 new commits

b06c60b Filtering: don't delete messages in DMs - MarkKoz
6eaabdb Merge branch 'master' into bug/filters/933/dont... - Den4200
55049f7 Merge pull request #934 from python-discord/bug... - Den4200

Den4200

[python-discord/bot] branch deleted: bug/filters/933/dont\-delete\-in\-dms

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200523.9 succeeded

Requested by

GitHub

Duration

00:02:49

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

MarkKoz

[bot:redis\_persistence] 1 new commit

a52a130 Remove redis session mock from MockBot - MarkKoz

[ebon magnet]

Build 20200523.10 succeeded

Requested by

GitHub

Duration

00:02:05

Build pipeline

Bot

[regal arch]

MarkKoz

[bot:redis\_persistence] 1 new commit

923d03a Show a warning when redis pool isn't closed - MarkKoz

[ebon magnet]

Build 20200523.11 succeeded

Requested by

GitHub

Duration

00:02:14

Build pipeline

Bot

[regal arch]

MarkKoz

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Thank you for explaining. I forgot about that as it's a bit counter-intuitive to return the same thing (it is convenient for testing of course). I suppose post.return_value.__aenter__.return_value = ... would have worked too then, right?

MarkKoz

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Thanks 😊

SebastiaanZ

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

SebastiaanZ

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

No, because the .post attribute is another mock than the mock it returns when you call it.

MarkKoz

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

So post.return_value != post()?

SebastiaanZ

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

SebastiaanZ

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

I misread, you're right.

[regal arch]

lemonsaurus

[bot:redis\_persistence] 4 new commits

b06c60b Filtering: don't delete messages in DMs - MarkKoz
6eaabdb Merge branch 'master' into bug/filters/933/dont... - Den4200
55049f7 Merge pull request #934 from python-discord/bug... - Den4200
96a3ac7 Merge branch 'master' into redis_persistence - lemonsaurus

kwzrd

[python-discord/seasonalbot] Pull request review submitted: #413 Encoding bug fix

Sorry I didn't get around to reviewing this sooner - I wanted to give myself some time to think about what we're really doing here w.r.t. the persistence module and files already existing in the production environment. Ultimately I think everything is fine since in production the encoding should always default to UTF-8 anyway, as far as I'm concerned, so files already existing there will still open fine.

I do agree with @Numerlor in that we should also specify the encoding when opening fil...

[ebon magnet]

Build 20200523.12 succeeded

Requested by

GitHub

Duration

00:02:07

Build pipeline

Bot

[regal arch]

MarkKoz

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

On the fence here, but these paragraphs should be less specific. I don't think they need to mention implementation details like the fact that it uses __set_name__ or that it relies on the Bot specifically for the redis pool. If you still want this information somewhere, then it's better to put it in some internal docs rather than the class's docstring.

[regal arch]

MarkKoz

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

This documentation doesn't appear to be accurate. NotImplementedError isn't raised anywhere. It's also confusing cause __init__ isn't directly raising any exceptions.

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

I would appreciate a comment here to explain what this loop does. It took me a moment to realise it keeps appending underscores until there's no longer a collision.

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Seems redundant to have "valid" in the name but I don't mind it that much.

MarkKoz

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

I don't know if this is a "wrong" way to write unit tests, but it's different from my approach. My problem with doing something like this is that your test not only relies on update working, but also clear and set. If set breaks, this test will fail even though it's only supposed to fail if update is broken (because it's meant to test update). It would be misleading to see this test fail for such reason because my first thought would be something is wrong with update, but upon c...

[regal arch]

Den4200

[python-discord/seasonalbot] New comment on issue #377: \.xkcd command to get xkcd comics

@CharlieADavies Are you still planning on working on this issue?

[regal arch]

MarkKoz

[bot:bug/filters/928/non\-ascii\-token] 1 new commit

2c7ff94 Token remover: escape dashes in regex - MarkKoz

[ebon magnet]

Build 20200524.1 failed

Requested by

GitHub

Duration

00:00:56

Build pipeline

Bot

[regal arch]

Numerlor

[python-discord/bot] New comment on issue #878: Further doc improvements

Alright to start working on the discussed features?

[regal arch]

CharlieADavies

[python-discord/seasonalbot] New comment on issue #377: \.xkcd command to get xkcd comics

This kinda fell off my radar due to other real life things; sorry for not updating the issue or commenting.

I can (and would like to) implement this, but I won't be able to for a little while - so if someone else wants to work on this I can't really object.

[north knot]

Postgres backup completed!

[regal arch]

aeros

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

aeros

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

I'm not 100% certain about the usage of a bare create_task() here for creating the redis session. To me, it seems like you would want to guarantee that the session is created before proceeding rather than scheduling the initialization to be ran in the background whenever it has the chance to (which is usually while other coroutines are being awaited, but there's no guarantee that it will have time to complete).

In order to guarantee its completion, you could do one of the following, depe...

MarkKoz

[python-discord/bot] New comment on issue #878: Further doc improvements

Yes, I think I've answered everything.

[regal arch]

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

We've been discussing on Discord about reworking the creation of all the objects here. Creating events for each thing is just too cumbersome. Your proposal to block the loop is quite a novel approach to me. Somehow, it never crossed my mind to do it like this.

I'm inclined to leave this as is and rework it along with creation of the other objects in a separate PR. Thoughts on this course of action?

MarkKoz

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

[regal arch]

aeros

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

aeros

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Your proposal to block the loop is quite a novel approach to me. Somehow, it never crossed my mind to do it like this.

Yeah, I personally think that for initialization purposes when startup/restart is highly infrequent and doesn't take especially long, it's perfectly okay for some calls to block the event loop. This can help simplify the startup process significantly, and the performance impact is usually negligible at best.

I'm inclined to leave this as is and rework it along with ...

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

I don't think I agree with that. Docstrings are internal docs. I think I have the opposite opinion here: I don't particularly see the need for some guide on our website to contain these sorts of implementation details, a guide should just explain how to use it. Docstrings, however, are for people who are digging into the source, and should provide a fuller idea of how the object works and behaves.

I should fix the wrong variable name, though.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Right, this was true once upon a time but definitely is not now.

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Good suggestion.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

using __name__ is a big improvement, yes. thanks!

lemonsaurus

[bot:redis\_persistence] 1 new commit

3f8dce7 use name for type list

• lemonsaurus

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

I see your point, but I'm not entirely sure how to test this properly without relying on set and clear to manage the state of the cache. Do you have a suggestion?

[ebon magnet]

Build 20200524.2 succeeded

Requested by

GitHub

Duration

00:02:08

Build pipeline

Bot

Build 20200524.3 succeeded

Requested by

GitHub

Duration

00:02:09

Build pipeline

Bot

[regal arch]

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

I thiiink I agree with this. Seems a bit hungarian. I'll remove it.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

I did consider that, but yeah, they're staticmethods out of concern for readability mostly.

Still, I'd be interested to hear more opinions on this. Anyone else feel these should be module constants?

lemonsaurus

[bot:redis\_persistence] 1 new commit

ed12a2f len(prefix) instead of hardcoding 2 - lemonsaurus

[ebon magnet]

Build 20200524.5 succeeded

Requested by

GitHub

Duration

00:02:04

Build pipeline

Bot

Build 20200524.4 succeeded

Requested by

GitHub

Duration

00:02:10

Build pipeline

Bot

[regal arch]

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Okay. This is an interesting bit of feedback, and thanks for that. I think I agree with what Mark is suggesting - that this should be refactored along with the other similar objects in a separate PR - so I'm going to resolve this conversation for now.

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Okay, as a compromise I've moved some of the specifics into the __set_name__ and __get__ docstrings, and simplified the stuff in the class docstring to this:

Please note that this class MUST be created as a class attribute, and that that class
must also contain an attribute with an instance of our Bot. See `__get__` and `__set_name__`
for more information about how this works.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

ks129

[python-discord/bot] New comment on pull request #947: RedisCache \- Data Persistence

Maybe should this have 2 more functions: incr and decr. These should useful for counters, because then you don't have to get value, increase it and then set. Also, maybe add some trace/info/debug logging?

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

After testing it out, I think it actually looks fine as a constant.

Let's go with that.

lemonsaurus

[bot:redis\_persistence] 2 new commits

98d8bb3 Refactor the nice prefix/type strings to constants - lemonsaurus
1d05a4d Improves various docstrings and comments. - lemonsaurus

[ebon magnet]

Build 20200524.6 succeeded

Requested by

GitHub

Duration

00:02:22

Build pipeline

Bot

[regal arch]

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Yep, this has now been refactored into returning an ItemsView, @MarkKoz

lemonsaurus

[python-discord/bot] New comment on pull request #947: RedisCache \- Data Persistence

Maybe should this have 2 more functions: incr and decr. These should useful for counters, because then you don't have to get value, increase it and then set. Also, maybe add some trace/info/debug logging?
@ks129 I think both of these suggestions are great! I'll work on that now.

[regal arch]

lemonsaurus

[bot:redis\_persistence] 3 new commits

f05fefb Better RuntimeErrors. - lemonsaurus
b2009d5 Make .items return ItemsView instead of AsyncIter - lemonsaurus
01bedca Add .increment and .decrement methods. - lemonsaurus

[ebon magnet]

Build 20200524.7 succeeded

Requested by

GitHub

Duration

00:02:08

Build pipeline

Bot

[regal arch]

lemonsaurus

[bot:redis\_persistence] 2 new commits

f80ce10 Rename Bot._redis_ready to Bot.redis_ready - lemonsaurus
361b740 Add logging to the RedisCache. - lemonsaurus

lemonsaurus

[python-discord/bot] New comment on pull request #947: RedisCache \- Data Persistence

All review comments should now have been addressed.

ks129

[python-discord/bot] New comment on pull request #947: RedisCache \- Data Persistence

Doesn't Redis have in-built commands for increasing and decreasing?

[ebon magnet]

Build 20200524.8 failed

Requested by

GitHub

Duration

00:01:44

Build pipeline

Bot

[regal arch]

SebastiaanZ

[python-discord/bot] New comment on pull request #947: RedisCache \- Data Persistence

Doesn't Redis have in-built commands for increasing and decreasing?

It does, but it does not work well with our current type string serialization system.

lemonsaurus

[python-discord/bot] New comment on pull request #947: RedisCache \- Data Persistence

Doesn't Redis have in-built commands for increasing and decreasing?

Yes - but we can't use these, because of how we're storing the data. Since we're converting all data into "typestrings" before it gets stored, Redis will not be able to increment numbers since they are stored as strings with a prefix.

lemonsaurus

[bot:redis\_persistence] 1 new commit

c5e6e8f MockBot needs to be aware of redis_ready - lemonsaurus

[ebon magnet]

Build 20200524.9 succeeded

Requested by

GitHub

Duration

00:02:00

Build pipeline

Bot

[regal arch]

lemonsaurus

[bot:redis\_persistence] 1 new commit

66d273f Add an option to use fakeredis in Bot. - lemonsaurus

[ebon magnet]

Build 20200524.10 succeeded

Requested by

GitHub

Duration

00:03:20

Build pipeline

Bot

[regal arch]

lemonsaurus

[python-discord/bot] New comment on pull request #947: RedisCache \- Data Persistence

I've added an option to config.yml called use_fakeredis in the latest commit, see 66d273f.

This will allow you guys to do a feature test of this without having to set up Redis. Just set this option to True, and the bot will use fakeredis instead of trying to connect to a Redis server.

This will make it a lot more contributor friendly, particularly for contributors on Windows. Redis is not even officially supported in Windows at all.

[regal arch]

MarkKoz

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

If I want to learn how to use this class, I will first read the class docstring to get an overview. My perspective is that knowing the implementation details is just noise at this point. I don't need to know that it specifically uses __set_name__ in order to learn to effectively use this class; I just need to know that it has to be instantiated as a class attribute.

I don't know what you mean by website guide. By internal docs, I meant moving this information to one of the "private" meth...

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Yeah - that's what I ended up doing, so I guess this is good then?

MarkKoz

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

One way to look at it is that we assume our redis library is already tested and working. So we can assume that if we use hmset_dict, it will work properly. The library already has tests written to ensure this. Thus, all our tests need to do is assert hmset_dict was called with the right arguments.

If not that, another way to do it is to use the redis command directly rather than via your class since, again, we assume these to be tested by the library.

On the other hand, these tests ...

MarkKoz

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

@aeros the fact that it's not a coroutine is cause it's following discord.py's design. If a non-coroutine is called by a coroutine, then I think it's fine (which is the case when login() calls this). But there's also clear(), which isn't a coroutine. And you can see I noted in a comment that calling it outside a coroutine will raise a warning. Discord.py creates its session outside a coroutine and supports re-creating it by calling clear() and I'm just going along with this design thoug...

MarkKoz

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Isn't KeyError more appropriate?

[regal arch]

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

I appreciate and understand your point. I think I will keep this in mind for future tests, but perhaps leave these to be for the time being, since it would require me to rewrite almost all the tests to achieve this sort of atomic testing at this point.

I think for our purposes, it's probably good enough for now that some of these tests start failing if there's a problem with the RedisCache. But I will try to write more atomic tests in the future.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

It absolutely is. Let's change this.

lemonsaurus

[bot:redis\_persistence] 1 new commit

ad8b1fa Improve error and error testing for increment - lemonsaurus

[ebon magnet]

Build 20200524.11 succeeded

Requested by

GitHub

Duration

00:02:07

Build pipeline

Bot

[regal arch]

SebastiaanZ

[python-discord/bot] Pull request review submitted: #956 Help System: Move dormant faster if help channel is empty

Tested locally and it works.

[ebon magnet]

Build 20200524.12 succeeded

Requested by

GitHub

Duration

00:01:15

Build pipeline

Bot

[regal arch]

SebastiaanZ

[python-discord/bot] Issue closed: #939 Shorten time till dormant after claimer deletes their message\(s\)

SebastiaanZ

[python-discord/bot] Pull request closed: #956 Help System: Move dormant faster if help channel is empty

SebastiaanZ

[bot:master] 10 new commits

6c5979b Config: Added new HelpChannels config delete... - ks129 [0c84302](https://github.com/python-discord/bot/commit/0c84302f7e3475c13924fda33c52e98566114082) Help: Implemented faster close when claimant de... - ks129 [a46eff8](https://github.com/python-discord/bot/commit/a46eff8d976cf65155f27ed75f49bd3e58155c84) Help: Fix docstrings of is_emptyandon_mess... - ks129
841ce9b Help: Create embed_description_match - ks129
d3550ce HelpChannels: fix is_empty not being awaited - MarkKoz

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200524.13 succeeded

Requested by

GitHub

Duration

00:03:02

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

Numerlor

[python-discord/bot] New comment on issue #878: Further doc improvements

I'll begin then

[regal arch]

n1kh1l1337

[python-discord/site] Fork created: n1kh1l1337/site

[regal arch]

jodth07

[python-discord/seasonalbot] New comment on pull request #413: Encoding bug fix

Sounds good, I will be updating this

[regal arch]

jodth07

[python-discord/seasonalbot] Issue opened: #414 partnerzodiac ext update with dates | or new zodiac ext

Description

Add additional parameters that can return

• the date based on zodiac sign
• zodiac sign based on dates.

Reasoning

I would like to for when .partnerzodiac is called with a zodiac sign,
In addition the zodiac info, we can also have the date range for the signs, such that those unfamiliar with the zodiac signs can learn about the dates as well.

Proposed Implementation

.partnerzodiac date
.partnerzodiac date virgo
The amazing people who are born b...

jos-b

[python-discord/bot] Issue opened: #958 Ban discord\.gift links

Within the community several discord.gift links are shared, half of them are trolls and this isn't worth keeping.

We'd like to add discord.gift links to the filter system so they are automatically deleted.

Links look like discord.gift/AABBCCDDEEFF and should be handled in a similar way to the way Discord invites are.

A lot of the code from the Discord invite filters can most likely be reused or refactored into a DRY approach to things to handle all links in this way.

An example ...

[regal arch]

lemonsaurus

[python-discord/bot] New comment on issue #958: Ban discord\.gift links

can't we just add discord.gift to the URL blacklist? Does this really need any code at all?

jos-b

[python-discord/bot] New comment on issue #958: Ban discord\.gift links

@lemonsaurus I believe so actually, I was under the misconception that the URL blacklist only checked for actual links (e.g. starting with a protocol), but that should work.

jos-b

[bot:master] 1 new commit

185c9e8 Add discord.gift to URL blacklist, closes #958 - jos-b

jos-b

[python-discord/bot] Issue closed: #958 Ban discord\.gift links

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200524.14 succeeded

Requested by

Joseph Banks

Duration

00:02:53

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

MarkKoz

[python-discord/bot] Issue opened: #959 Re\-think approach for creating sessions in the Bot

Background

How sessions are currently created

There are things the Bot needs to create which rely on a running event loop like aiohttp ClientSessions. This is done in Bot._recreate():

https://github.com/python-discord/bot/blob/b25619567d5d1ceee939d4b17c1a90ae9ac337bf/bot/bot.py#L87-L117

Because discord.py manages the event loop, we currently hook into Bot.login() and create everything in there. This is done indirectly by calling _recreate() within login():

ht...

[ebon magnet]

Build 20200525.1 succeeded

Requested by

GitHub

Duration

00:00:56

Build pipeline

Seasonal Bot

Build 20200525.2 succeeded

Requested by

GitHub

Duration

00:01:02

Build pipeline

Seasonal Bot

[regal arch]

Lvl4Sword

[python-discord/site] Issue opened: #354 Create robots\.txt file

Python Discord staff tend to check terms of service and robots.txt to see if a site can be scraped.
I feel it'd be wise to implement a robots.txt file for the Python Discord site to lead by example.

Lvl4Sword

[python-discord/site] Issue opened: #355 Fix issues in https://web\.dev/measure

According to https://web.dev/measure , the Python Discord site currently scores 74/100 in Performance, 86/100 in Accessibility, 100/100 in Best Practices, and 91/100 in SEO.
These seem like some relatively easy things to implement for the speed ( and other ) bonuses they offer.

[regal arch]

aeros

[python-discord/bot] New comment on issue #959: Re\-think approach for creating sessions in the Bot

@SebastiaanZ had concerns of whether the object created would be fine with the fact that the loop would temporarily stop running after they're created. There'd be a gap in which the loop is stopped between when the objects are finished being created and when discord.py starts the loop back up to run the bot.

Although I have not specifically tried this w/ aiohttp, I don't believe this will be an issue as long as the event loop is not closed during the "gap". It's definitely worth some exp...

[north knot]

Postgres backup completed!

[regal arch]

kwzrd

[python-discord/seasonalbot] Pull request review submitted: #413 Encoding bug fix

Looks excellent, @jodth07! Thanks a lot for handling it.

I pushed a small commit to fix up a typo, didn't want to bother you with it.

Tested locally, all works, and I couldn't find any remaining opens with unspecified encoding, so let's go ahead and merge this.

[ebon magnet]

Build 20200525.3 succeeded

Requested by

GitHub

Duration

00:01:02

Build pipeline

Seasonal Bot

[regal arch]

kwzrd

[python-discord/seasonalbot] Issue closed: #334 Cleanup: Add explicit encoding on open\(\) calls

kwzrd

[python-discord/seasonalbot] Issue closed: #411 Files without an encoding specified may crash bot

kwzrd

[python-discord/seasonalbot] Pull request closed: #413 Encoding bug fix

kwzrd

[seasonalbot:master] 8 new commits

16a28ac 5/19 - evergreen | triva - fixed utf-8 bugs for... - jodth07
9c0cbed 5/19 - bot | xmas, easter, evergreen - updated ... - jodth07
ec1604d 5/19 - ext - set upen files encoding to utf8 fo... - jodth07
86dad21 5/24 - bot exts - updated writing with utf8 enc... - jodth07
de7e39a 5/24 - bot utils - added encoding, and notes fo... - jodth07

[north knot]

Auto-deployed: seasonalbot

[ebon magnet]

Build 20200525.4 succeeded

Requested by

GitHub

Duration

00:02:15

Build pipeline

Seasonal Bot

[night lily]

SeasonalBot

Connected!

[regal arch]

kwzrd

[python-discord/seasonalbot] New comment on pull request #386: Bookmark

I will close this due to inactivity, @Akarys42 is now assigned on both linked issues.

kwzrd

[python-discord/seasonalbot] Pull request closed: #386 Bookmark

kwzrd

[python-discord/seasonalbot] New comment on pull request #365: Connect Four Game

Hello, what is the status here? Looks like there is some feedback that needs to be addresed, do you think you will be able to do that, @Suhail6inkling?

[native jolt]

dependabot-preview[bot]

[flake8-annotations:dependabot/pip/flake8\-3\.8\.2] 1 new commit

e7cdbaa Bump flake8 from 3.7.9 to 3.8.2 - dependabot-preview[bot]

dependabot-preview[bot]

[python-discord/flake8-annotations] Pull request opened: #86 Bump flake8 from 3\.7\.9 to 3\.8\.2

Bumps flake8 from 3.7.9 to 3.8.2.

Commits

4071645 Release 3.8.2
b9fe4d6 Merge branch 'extend_exclude_is_files' into 'master'
31c2f9f treat --extend-exclude as a file list
d95f9cf Merge branch 'opt-config-relative-to' into 'master'
563220b config: Normalize paths in CLI-specified config relative to config dir
8be5a72 config: Normalize paths in config relative to provided parent directory
c8494e7 Merge branch 'master' into 'master'
4557357 Parse --jobs as a ...

dependabot-preview[bot]

[python-discord/flake8-annotations] New branch created: dependabot/pip/flake8\-3\.8\.2

[native jolt]

sco1

[python-discord/flake8-annotations] New comment on pull request #86: Bump flake8 from 3\.7\.9 to 3\.8\.2

Can be handled by #83

sco1

[python-discord/flake8-annotations] Pull request closed: #86 Bump flake8 from 3\.7\.9 to 3\.8\.2

dependabot-preview[bot]

[python-discord/flake8-annotations] New comment on pull request #86: Bump flake8 from 3\.7\.9 to 3\.8\.2

OK, I won't notify you again about this release, but will get in touch when a new version is available.

If you change your mind, just re-open this PR and I'll resolve any conflicts on it.

dependabot-preview[bot]

[python-discord/flake8-annotations] branch deleted: dependabot/pip/flake8\-3\.8\.2

[native jolt]

Dreamsorcerer

[python-discord/flake8-annotations] Issue opened: #87 Optional return type for \_\_init\_\_\(\)

For consistency with mypy, it would be nice to have an option/rule that behaved the same way as mypy regarding typing of init().

Currently, flake8-annotations always raises a violation if there is no return type annotation.

Mypy, however, raises an error if there are no annotations (as this would indicate an untyped function).
i.e. If there are any annotations on parameters, then a return type is not needed. The return type is only needed when there are no parameters in order to ma...

sco1

[python-discord/flake8-annotations] New comment on issue #87: Optional return type for \_\_init\_\_\(\)

Suppressing ANN204 will ignore these warnings.

Dreamsorcerer

[python-discord/flake8-annotations] New comment on issue #87: Optional return type for \_\_init\_\_\(\)

Yes, but it doesn't give any violation for def __init__(self):, which Mypy does. Allowing or suppressing will result in different behaviour to Mypy (more lenient or more relaxed respectively).

So, to be consistent with Mypy, I'm expecting violations for:
def __init__(self):
but, no violations for:
def __init__(self, foo: str):

Dreamsorcerer

[python-discord/flake8-annotations] New comment on issue #87: Optional return type for \_\_init\_\_\(\)

i.e. Mypy assumes an implicit return type of None for __init__(). But, there has to atleast 1 annotation somewhere in order for the function to be marked as statically typed. No annotations indicates a dynamically typed function.
So, standard practice for def __init__(self): is to add a return type annotation to mark it as statically typed.

[native jolt]

sco1

[python-discord/flake8-annotations] New comment on issue #87: Optional return type for \_\_init\_\_\(\)

Ok, I'm following now, sorry.

Right now we don't have a mechanism for discriminating between statically and dynamically typed functions like Mypy does (not just for __init__). Adding something like --allow-untyped-defs a la Mypy's --disallow-untyped-defs shouldn't be too complicat...

Dreamsorcerer

[python-discord/flake8-annotations] New comment on issue #87: Add Flag to Suppress Warnings for Dynamically Typed Functions

Indeed, I actually just disabled that flag, as Mypy will complain about no return types on regular functions. It's only __init__() that gets an implicit return type.

[oak estuary]

Python Bot

Connected!

[regal arch]

kwzrd

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

kwzrd

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

I think the exception method is misused here (and throughout the cog). As per the logging docs, it's a convenience method for automatically appending the traceback when logging exceptions. It shouldn't be used outside of an except block. We should log using error directly, as there is no traceback here.

[native jolt]

sco1

[flake8-annotations:suppress\-dynamically\-typed\-defs] 2 new commits

5d37fe5 Add flag for suppressing all errors from dynami... - sco1
6fa2199 Add tests for error suppression for dynamically... - sco1

sco1

[python-discord/flake8-annotations] New branch created: suppress\-dynamically\-typed\-defs

[flake8-annotations] Azure Pipelines checks success on suppress\-dynamically\-typed\-defs

[regal arch]

kwzrd

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

kwzrd

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

What is the motivation for the log? Wouldn't it be better to have the entire message in the exception that is raised on the line below?

[native jolt]

sco1

[flake8-annotations:dev\-next] 4 new commits

5d37fe5 Add flag for suppressing all errors from dynami... - sco1
6fa2199 Add tests for error suppression for dynamically... - sco1
8f17ffd Merge branch 'suppress-dynamically-typed-defs' ... - sco1
f3cdd85 Bump ver - sco1

sco1

[python-discord/flake8-annotations] branch deleted: suppress\-dynamically\-typed\-defs

[regal arch]

kwzrd

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

kwzrd

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

This can give None so return type should be annotated as Optional.

kwzrd

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Not sure about this - what if I happen to provide the same default as the fetched value? Can't that happen?

kwzrd

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

If I understand this correctly, the items param could be annotated as Dict[RedisType, ReditType].

[regal arch]

kwzrd

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

kwzrd

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

I'm aware that this mimics a dict update, but maybe the docstring could be a little bit more detailed? If you think it's fine as it is, then it can stay.

I didn't instantly understand what this will do from the function signature & docstring, but I also didn't connect it with a dict at first.

[regal arch]

kwzrd

[python-discord/bot] New comment on pull request #947: RedisCache \- Data Persistence

I played with this a little bit today and it seems to work really well - I think this PR is very close to being ready. Redis worked for me right out of the box with no extra set-up needed (using Docker).

I tested by setting up my own cog that uses two RedisCache instances. The namespace resolution is very cool - I was worried a little bit earlier but that was mainly due to the "3 allowed ways to instantiate" part. I don't mind awaiting the getters and setters at all. Trace-level logs hel...

[ebon magnet]

Build 20200525.1 succeeded

Requested by

GitHub

Duration

00:01:20

Build pipeline

Bot

[regal arch]

MrGrote

[python-discord/bot] Pull request review submitted: #930 Add tests for the antimalware cog

MrGrote

[python-discord/bot] New review comment on pull request #930: Add tests for the antimalware cog

I had not considered this, pushed the changes.

MarkKoz

[bot:redis\_persistence] 4 new commits

856cecb Add support for Union type annotations for cons... - MarkKoz
9b9aa9b Support validating collection types for constants - MarkKoz
87d42ad Improve output of section name in config valida... - MarkKoz
8b5c1aa Expose the redis port to the host - MarkKoz

[ebon magnet]

Build 20200525.2 succeeded

Requested by

GitHub

Duration

00:02:06

Build pipeline

Bot

[regal arch]

MarkKoz

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

Here is a cog I wrote to test this out. All the functions work!

That being said, I had trouble trying to forcefully trigger some of the errors in __get__ and _validate_cache. Whether I created the cache as an instance attribute, local variable in a function, or a module variable, the error was always that the bot is missing.

I wanted to try to trigger this error

https://github.com/python-discord/bot/blob/redis_persistence/bot/utils...

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

This should be private.

[regal arch]

MarkKoz

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Let's drop "class" cause it may imply to some that instance attributes aren't acceptable.

                "attribute."

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

The order of these two checks should be swapped. If I assign the cache to an instance attribute and then try to use it, it will still complain there's no bot even if my class has one. This makes it confusing cause the real issue is that my cache isn't a class attribute.

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Let's drop "class" cause it may imply to some that instance attributes aren't acceptable.

                "attribute."

MarkKoz

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

MarkKoz

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Being phrased as a question, it isn't clear if a class attribute is the cause of the error or the solution i.e. "Did you do this? Well, you shouldn't have!" vs. "Did you do this? You should have.". I think a statement would be clearer e.g. " You must do x..."

[regal arch]

kwzrd

[python-discord/bot] Pull request review submitted: #936 Filtering: don't attempt to send additional embeds for invalid invites

Good fix! Let's get this merged.

The cog looks like it could use a rewrite - but let's leave that for another day.

kwzrd

[bot:bug/filters/929/invalid\-invites] 122 new commits

05616ae Refactor the Help command. - mathsman5133
87f9fdd Minor formatting changes to align with current ... - mathsman5133
5ebb87d Add a special case for when the help command in... - mathsman5133
79d5b2d Few changes to keep formatting same as current - mathsman5133
d8384b2 Show a maximum of 8 commands per page rather th... - mathsman5133

[ebon magnet]

Build 20200525.3 succeeded

Requested by

GitHub

Duration

00:01:18

Build pipeline

Bot

[regal arch]

kwzrd

[python-discord/bot] Issue closed: #929 Invite Filter Raises on Invalid Invites

kwzrd

[python-discord/bot] Pull request closed: #936 Filtering: don't attempt to send additional embeds for invalid invites

kwzrd

[bot:master] 3 new commits

55b8aed Filtering: don't attempt to send additional emb... - MarkKoz
d497b76 Merge branch 'master' into bug/filters/929/inva... - kwzrd
0e26ee3 Merge pull request #936 from python-discord/bug... - kwzrd

kwzrd

[python-discord/bot] branch deleted: bug/filters/929/invalid\-invites

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200525.4 succeeded

Requested by

GitHub

Duration

00:03:03

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

jos-b

[bot:master] 1 new commit

6cedfdc [stats] Do not report modmail channels to stats - jos-b

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200526.1 succeeded

Requested by

Joseph Banks

Duration

00:03:24

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[north knot]

Postgres backup completed!

[regal arch]

ikuyarihS

[bot:restricted\_tags] 422 new commits

05616ae Refactor the Help command. - mathsman5133
87f9fdd Minor formatting changes to align with current ... - mathsman5133
5ebb87d Add a special case for when the help command in... - mathsman5133
79d5b2d Few changes to keep formatting same as current - mathsman5133
d8384b2 Show a maximum of 8 commands per page rather th... - mathsman5133

[ebon magnet]

Build 20200526.2 succeeded

Requested by

GitHub

Duration

00:01:18

Build pipeline

Bot

[regal arch]

ikuyarihS

[python-discord/bot] Issue closed: #778 Role restricted tags

ikuyarihS

[python-discord/bot] Pull request closed: #866 feature to restrict tags to specific role\(s\)

ikuyarihS

[bot:master] 8 new commits

72768b4 Add feature to restrict tags to specific role(s) - RohanJnr
00d22a3 Remove unnecessary variable creation and join t... - RohanJnr
7195a92 Feature now is cross-platform - RohanJnr
0b71904 simplify if statement - RohanJnr
8bf1df9 simpl - RohanJnr

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200526.3 succeeded

Requested by

GitHub

Duration

00:03:07

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

ikuyarihS

[python-discord/bot] Issue opened: #960 Update \`\!free\` tag

Problem

With the introduction of #how-to-get-help - we can safely update the content of the !free tag, right now the content is this:

Proposal

Let's include the id of the #how-to-get-help into the tag

The tag can be found here:
https://github.com/python-discord/bot/blob/master/bot/resources/tags/free.md

[regal arch]

kwzrd

[python-discord/bot] New comment on issue #960: Update \`\!free\` tag

That's a good idea. I'll try to figure something out.

[regal arch]

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

RuntimeErrors are supposed to help you figure out what you're doing wrong when you're trying to implement the RedisCache.

These logging events are supposed to help us find and diagnose bugs via Sentry.

I would be okay with including the full message, but I guess the motivation was that RuntimeErrors are contributor-facing while the exceptions are mostly just for core devs, who wouldn't mind the brevity.

Anyway, I'll change exceptions into errors and use the same error message.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

damn, that's true. I hadn't considered that. Nice catch!

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

just gonna remove this condition - await self.delete(key) will just do nothing if the key doesn't exist, and I can't think of a way to prevent this API call that doesn't involve another API call
(or some horrible form of "caching the cache") anyway.

[regal arch]

lemonsaurus

[bot:redis\_persistence] 7 new commits

9b882c7 Turn log.exception into log.error - lemonsaurus
723c1d3 Fix edge case where pop might not delete. - lemonsaurus
b630ace Add better docstring for RedisCache.update - lemonsaurus
1920673 Make self.increment_lock private. - lemonsaurus
46a377d Improve some docstrings for RedisCache. - lemonsaurus

lemonsaurus

[bot:redis\_persistence] 24 new commits

72768b4 Add feature to restrict tags to specific role(s) - RohanJnr
00d22a3 Remove unnecessary variable creation and join t... - RohanJnr
7195a92 Feature now is cross-platform - RohanJnr
0b71904 simplify if statement - RohanJnr
8bf1df9 simpl - RohanJnr

[ebon magnet]

Build 20200526.5 succeeded

Requested by

GitHub

Duration

00:01:59

Build pipeline

Bot

[regal arch]

MarkKoz

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

[regal arch]

Den4200

[bot:master] 1 new commit

63a922e Add /r/FlutterDev to the guild invite whitelist - Den4200

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200526.6 succeeded

Requested by

GitHub

Duration

00:03:29

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

ks129

[python-discord/bot] New comment on issue #856: Detecting bad words in nicknames

I think this should listen for on_member_update event, not on_message because on_message will be called too much. on_member_update should check is display_name changed and when it is, do check.

[regal arch]

lemonsaurus

[bot:moderation\_commands\_in\_modmail\_category] 5 new commits

d9190d9 Refactor the in_whitelist deco to a check. - lemonsaurus
b51c206 Allow infraction management in modmail category - lemonsaurus
d310f42 Find + change all InWhitelistCheckFailure imports - lemonsaurus
7562262 No redirect for mod management. - lemonsaurus
c3cbc84 Allow some commands to fail checks silently. - lemonsaurus

lemonsaurus

[python-discord/bot] New branch created: moderation\_commands\_in\_modmail\_category

lemonsaurus

[python-discord/bot] Pull request opened: #961 Permit moderation commands in ModMail category

This pull request allows commands like !infr search inside any channel in the new ModMail category.

It does this by refactoring the in_whitelist decorator into an in_whitelist_check and making the in_whitelist deco simply use this check. Then we also use that check in the moderation/management.py's cog_check function, asking it to check that anyone using this cog has a moderator role, is in one of the moderation channels, or is in any channel inside the ModMail category.

The...

[ebon magnet]

Build 20200527.1 succeeded

Requested by

GitHub

Duration

00:01:20

Build pipeline

Bot

[north knot]

Postgres backup completed!

[regal arch]

eivl

[python-discord/bot] Pull request review submitted: #961 Permit moderation commands in ModMail category

eivl

[python-discord/bot] New review comment on pull request #961: Permit moderation commands in ModMail category

Clever, but do we not log people using secret commands? maybe just to see if it's misused in some way?

lemonsaurus

[python-discord/bot] Pull request review submitted: #961 Permit moderation commands in ModMail category

lemonsaurus

[python-discord/bot] New review comment on pull request #961: Permit moderation commands in ModMail category

Not currently. Or, well, there is a trace log a few lines above this but that won't make it to Sentry.

I believe only actionable programming errors should be in Sentry, so I don't think I want this. it doesn't really matter if regular users try to call this.

[regal arch]

SebastiaanZ

[python-discord/bot] Pull request review submitted: #961 Permit moderation commands in ModMail category

SebastiaanZ

[python-discord/bot] New review comment on pull request #961: Permit moderation commands in ModMail category

InWhitelistCheckFailure should never make it to Sentry (the error handler handles them), but they do send a message back to the context in which the command was issued. This check makes sure we don't do that for moderation commands and the like (which is also the current behavior with the in_channel_check we are replacing).

lemonsaurus

[python-discord/bot] Pull request review submitted: #961 Permit moderation commands in ModMail category

lemonsaurus

[python-discord/bot] New review comment on pull request #961: Permit moderation commands in ModMail category

Yeah - without this, if someone tries to do a !infr search in a regular channel, it'll show this in the channel where you ran the command

[regal arch]

lemonsaurus

[site:remove\_avatar\_hash] 1 new commit

354d78e No longer accept or track avatar_hash. - lemonsaurus

lemonsaurus

[python-discord/site] New branch created: remove\_avatar\_hash

lemonsaurus

[python-discord/site] Pull request opened: #356 No longer accept or track \`avatar\_hash\`

This should completely remove avatar_hash from the site - both in our tests, in the model itself, and from the database (as a result of the migration).

This should be merged at the same time as the equivalent bot PR, which I am going to open very soon.

[ebon magnet]

Build 20200527.1 succeeded

Requested by

GitHub

Duration

00:02:42

Build pipeline

Site

[regal arch]

SebastiaanZ

[python-discord/bot] Pull request review submitted: #961 Permit moderation commands in ModMail category

I've got one comment about the noqa's you've added.

Other than that, just a general observation: The "in_whitelist" check/decorator combo is slowly taking over the locations where we currently use in_channel. I know that there was some initial resistance to the idea, but I think we should consider unifying it across the code base to make things consistent.

SebastiaanZ

[python-discord/bot] New review comment on pull request #961: Permit moderation commands in ModMail category

I'm probably missing something here, but why did you add these? I'm a bit surprised that this error code would pop-up here (unless PyFlakes does not understand allowed_strings with arguments) and I can't reproduce it locally with our linting set-up and these noqa's removed.

        infraction_id: t.Union[int, allowed_strings("l", "last", "recent")],
        duration: t.Union[Expiry, allowed_strings("p", "permanent"), None],

lemonsaurus

[bot:stop\_sending\_avatar\_hash] 1 new commit

7230449 Remove all sending of avatar_hash. - lemonsaurus

lemonsaurus

[python-discord/bot] New branch created: stop\_sending\_avatar\_hash

lemonsaurus

[python-discord/bot] Pull request opened: #962 Remove all sending of avatar\_hash

No longer send avatar_hash to the API at any point. We don't want to keep storing it.

This depends on this PR to be merged:
https://github.com/python-discord/site/pull/356

[ebon magnet]

Build 20200527.2 failed

Requested by

GitHub

Duration

00:01:07

Build pipeline

Bot

[regal arch]

lemonsaurus

[bot:stop\_sending\_avatar\_hash] 1 new commit

8e0cdb2 Fix failing tests related to avatar_hash - lemonsaurus

[ebon magnet]

Build 20200527.3 succeeded

Requested by

GitHub

Duration

00:01:22

Build pipeline

Bot

[regal arch]

lemonsaurus

[python-discord/bot] Pull request review submitted: #961 Permit moderation commands in ModMail category

lemonsaurus

[python-discord/bot] New review comment on pull request #961: Permit moderation commands in ModMail category

It's a new flake8 3.8 error, that's probably why you can't reproduce it. I agree that it's basically a bug, but it does fail the linter at 3.8.

lemonsaurus

[python-discord/bot] New comment on pull request #961: Permit moderation commands in ModMail category

Other than that, just a general observation: The "in_whitelist" check/decorator combo is slowly taking over the locations where we currently use in_channel. I know that there was some initial resistance to the idea, but I think we should consider unifying it across the code base to make things consistent.

Ah yeah, I was gonna do this. I think this PR is a good time to do it, too, so I'll commit a fix a little later today removing in_channel and replacing all instances of it with `in_wh...

aeros

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

aeros

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Without knowing how exactly the Redis Cache will be used, this may be a bit hard to predict, but do you have a general estimate of how frequently the caller might attempt to increment a key that doesn't exist? If it's expected to be reasonably often, it may be worth releasing the lock immediately after the if value is None: check.

However, if this is more for API safety and it's expected that the caller will be using a key that definitely exists 99% of the time, it's probably fine as it is.

lemonsaurus

[python-discord/bot] Pull request review submitted: #961 Permit moderation commands in ModMail category

lemonsaurus

[python-discord/bot] New review comment on pull request #961: Permit moderation commands in ModMail category

Also, I know this isn't strictly relevant to this PR, it's here out of pure laziness because I didn't wanna downgrade my flake8 and pre-commit setup. but if you mind, I could.

aeros

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

aeros

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

I would suggest also having a test_increment_concurrent() to ensure it's actually safe from race conditions. It should be okay with the current lock implementation, but to actually test for it I'd advise something like the following:

await self.redis.set("test_key", 0)
tasks = []
# Feel free to adjust the count if needed
for _ in range(100):
    task = asyncio.create_task(
        self.redis.increment("test_key", 1))
    tasks.append(task)

await asyncio.gather(*tasks)
v...

SebastiaanZ

[python-discord/bot] Pull request review submitted: #961 Permit moderation commands in ModMail category

SebastiaanZ

[python-discord/bot] New review comment on pull request #961: Permit moderation commands in ModMail category

We're going to merge a PR that includes a relock soon-ish, right?

It would make more sense to document this in that PR so it's clear why this was changed, but it feels a bit like creating a lot of work (removing it here, adding it back in another PR).

Do you know which PR is merging that version upgrade? It'll be good to keep an eye on if it ran a full lint over the entire code or just the files changed. If the latter, it may create confusing linting errors for future contribs, as linti...

SebastiaanZ

[python-discord/bot] Pull request review submitted: #961 Permit moderation commands in ModMail category

[regal arch]

j03b

[python-discord/bot] Pull request review submitted: #961 Permit moderation commands in ModMail category

j03b

[python-discord/bot] Pull request review submitted: #962 Remove all sending of avatar\_hash

j03b

[bot:moderation\_commands\_in\_modmail\_category] 2 new commits

63a922e Add /r/FlutterDev to the guild invite whitelist - Den4200
0dc1e5f Merge branch 'master' into moderation_commands_... - j03b

[ebon magnet]

Build 20200527.4 succeeded

Requested by

GitHub

Duration

00:01:25

Build pipeline

Bot

[regal arch]

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

do you have a general estimate of how frequently the caller might attempt to increment a key that doesn't exist?

There is no reason this should ever happen outside of the local environment. If someone wrote a feature that did this, we would probably catch it during review and prevent it from ever being merged. This feature won't be directly exposed to users (at least, I really don't think it should be), so in my opinion this should practically never happen.

lemonsaurus

[python-discord/bot] Pull request review submitted: #961 Permit moderation commands in ModMail category

lemonsaurus

[python-discord/bot] New review comment on pull request #961: Permit moderation commands in ModMail category

The full relint is handled in https://github.com/python-discord/bot/pull/947, and yes, it is a full relint.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

oh yeah, I really like this. I'll set that up now. We can probably go even higher than 100, I doubt it'll add much test time if we go for 1000, and I'd feel a bit safer with that.

[regal arch]

kwzrd

[python-discord/bot] New comment on pull request #947: RedisCache \- Data Persistence

During my testing yesterday I also found out that the test cases aren't entirely isolated in the sense that values in the cache persist between them. My approach would have been to make sure the cache is cleared before each test (probably in the setUp), but I don't require that this is done. I just thought I'd mention this in case it's not intentional.

aeros

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

aeros

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Awesome! Yeah, 1000 or so should more fully cover your bases; I just had no idea what the actual runtime was like so I used 100 as a more conservative estimate. I'm glad the example was helpful though. :-)

aeros

[python-discord/bot] New comment on pull request #947: RedisCache \- Data Persistence

@kwzrd

My approach would have been to make sure the cache is cleared before each test (probably in the setUp), but I don't require that this is done.

I agree that it would make sense to clear the cache in between tests, otherwise it ends up being somewhat subject to side effects from previous tests and can lead to inconsistent results depending on the order the tests are executed in (which is typically something to avoid).

lemonsaurus

[bot:redis\_persistence] 1 new commit

35a1de3 Clear cache in asyncSetUp instead of tests. - lemonsaurus

lemonsaurus

[python-discord/bot] New comment on pull request #947: RedisCache \- Data Persistence

My approach would have been to make sure the cache is cleared before each test (probably in the setUp)

Addressed in 35a1de3

[ebon magnet]

Build 20200527.5 succeeded

Requested by

GitHub

Duration

00:03:17

Build pipeline

Bot

[regal arch]

lemonsaurus

[bot:redis\_persistence] 1 new commit

b189307 Refactor .increment and add lock test. - lemonsaurus

[ebon magnet]

Build 20200527.6 succeeded

Requested by

GitHub

Duration

00:02:28

Build pipeline

Bot

[ebon magnet]

Build 20200527.7 failed

Requested by

GitHub

Duration

00:01:14

Build pipeline

Bot

[regal arch]

lemonsaurus

[bot:moderation\_commands\_in\_modmail\_category] 2 new commits

db0a384 Remove the now deprecated in_channel_check. - lemonsaurus
876fae1 Add some tests for in_whitelist_check. - lemonsaurus

lemonsaurus

[python-discord/bot] New comment on pull request #961: Permit moderation commands in ModMail category

I'll commit a fix a little later today removing in_channel and replacing all instances of it with in_whitelist.

This was handled in db0a384

I also added some tests for the new in_whitelist_check in 876fae1.

I think this is probably ready to merge now.

[ebon magnet]

Build 20200527.8 succeeded

Requested by

GitHub

Duration

00:18:57

Build pipeline

Bot

[regal arch]

ks129

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

This looks very good for me. I see so much use cases for this.

[regal arch]

lemonsaurus

[python-discord/site] Pull request review submitted: #348 Off\-topic channel names non random selecting

So, I think the overall approach (tracking names that are used with a boolean and setting everything to used=False when you run out) is the right approach. However, I don't think I see any need for this to be a parameter. This should just be the way that it always works. Why would we ever want to return names in any other way than this?

Additionally, making this change would require a change on the bot side to make use of this new optional param.

Please refactor this so that this new ro...

lemonsaurus

[python-discord/site] New review comment on pull request #348: Off\-topic channel names non random selecting

            field=models.BooleanField(default=False, help_text='Whether or not this name has already been used during this rotation'),

lemonsaurus

[python-discord/site] New review comment on pull request #348: Off\-topic channel names non random selecting

Please add docstrings to your test functions, it makes them display better when we run them.

lemonsaurus

[python-discord/site] New review comment on pull request #348: Off\-topic channel names non random selecting

        help_text="Whether or not this name has already been used during this rotation",

ks129

[python-discord/site] Pull request review submitted: #348 Off\-topic channel names non random selecting

ks129

[python-discord/site] New review comment on pull request #348: Off\-topic channel names non random selecting

I'm pretty sure that this should used=False because then name is again available.

lemonsaurus

[python-discord/site] Pull request review submitted: #348 Off\-topic channel names non random selecting

lemonsaurus

[python-discord/site] New review comment on pull request #348: Off\-topic channel names non random selecting

oh, yeah. my bad.

Den4200

[python-discord/bot] Pull request review submitted: #961 Permit moderation commands in ModMail category

SebastiaanZ

[python-discord/bot] New comment on pull request #961: Permit moderation commands in ModMail category

With four approvals, this should be ready to merge.

SebastiaanZ

[python-discord/bot] Pull request closed: #961 Permit moderation commands in ModMail category

SebastiaanZ

[bot:master] 9 new commits

d9190d9 Refactor the in_whitelist deco to a check. - lemonsaurus
b51c206 Allow infraction management in modmail category - lemonsaurus
d310f42 Find + change all InWhitelistCheckFailure imports - lemonsaurus
7562262 No redirect for mod management. - lemonsaurus
c3cbc84 Allow some commands to fail checks silently. - lemonsaurus

SebastiaanZ

[python-discord/bot] branch deleted: moderation\_commands\_in\_modmail\_category

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200527.9 succeeded

Requested by

GitHub

Duration

00:04:32

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

lemonsaurus

[bot:redis\_persistence] 1 new commit

4db313e Floats are no longer permitted as RedisCache keys. - lemonsaurus

[ebon magnet]

Build 20200527.10 succeeded

Requested by

GitHub

Duration

00:02:19

Build pipeline

Bot

[regal arch]

lemonsaurus

[bot:redis\_persistence] 1 new commit

b6093bf Refactor typestring converters to partialmethods. - lemonsaurus

lemonsaurus

[bot:redis\_persistence] 1 new commit

63b81b0 Fix ATROCIOUS comment. - lemonsaurus

lemonsaurus

[bot:redis\_persistence] 11 new commits

63a922e Add /r/FlutterDev to the guild invite whitelist - Den4200
d9190d9 Refactor the in_whitelist deco to a check. - lemonsaurus
b51c206 Allow infraction management in modmail category - lemonsaurus
d310f42 Find + change all InWhitelistCheckFailure imports - lemonsaurus
7562262 No redirect for mod management. - lemonsaurus

[ebon magnet]

Build 20200527.13 succeeded

Requested by

GitHub

Duration

00:02:10

Build pipeline

Bot

[regal arch]

aeros

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

aeros

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

In the case where there are multiple of the same exception class which can be raised from the same method (such as w/ cache.get()), I would recommend using assertRaisesRegex to ensure it causes the correct exception message.

[regal arch]

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

I don't know if I agree. This kind of testing is very brittle. I don't really like the idea that changing a string should fail a test, especially for a project like a community bot.

If it was absolutely crucial that we arrive at the right RuntimeError, I would, but I really don't think it is. As long as these raise errors at all, it's really all fine.

[regal arch]

lemonsaurus

[bot:redis\_persistence] 1 new commit

bdb7bbc Reduce complexity on some of the typestring stuff. - lemonsaurus

[ebon magnet]

Build 20200527.14 succeeded

Requested by

GitHub

Duration

00:02:15

Build pipeline

Bot

[regal arch]

kwzrd

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

Looks good. I did a function review yesterday & a smaller one today after the changes and everything seems to work fine. The real test will happen once a cog actually begins using the cache. All my comments were addressed so I'm happy to approve. Solid work @lemonsaurus!

Before merge, let's wait for @SebastiaanZ's approval.

lemonsaurus

[bot:redis\_persistence] 2 new commits

11542fb Make prefix consts private and more precise. - lemonsaurus
f66a635 Add custom exceptions for each error state. - lemonsaurus

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Adressed in a slightly different way in f66a635

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

[ebon magnet]

Build 20200527.15 succeeded

Requested by

GitHub

Duration

00:02:06

Build pipeline

Bot

[regal arch]

MarkKoz

[python-discord/bot] Issue opened: #963 Emoji in help channel name sometimes doesn't change when it become occupied

For example:

This needs to be investigated. Current cause is unknown.

Numerlor

[python-discord/bot] New comment on issue #963: Emoji in help channel name sometimes doesn't change when it becomes occupied

I believe the channel also didn't get moved to the end of the channel list, but started at the top instead if that could be related

aeros

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

aeros

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

I don't know if I agree. This kind of testing is very brittle. I don't really like the idea that changing a string should fail a test, especially for a project like a community bot.

Fair enough. I mostly come from experience w/ the exception messages remaining static for long periods of time (if not forever), so the string for the exception message changing isn't an issue.

The custom exception classes are another good approach, and I'd say it probably makes more sense in this case sin...

[regal arch]

aeros

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

aeros

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

I think there's a bit of an issue with relying on await self.bot.redis_ready.wait() after the redis session is closed in Bot.close(). Specifically, when the bot calls self.redis_ready.clear(), it sets the condition to False, and with how Condition.wait() works, it could end up indefinitely suspending the coroutine while waiting for the condition to be set to True. This would be an issue if _validate_cache() was called during or after Bot.close().

I'm not overly concerned about ...

[regal arch]

[site] Azure Pipelines checks failure on e321a25

[ebon magnet]

Build 20200528.1 failed

Requested by

GitHub

Duration

00:01:56

Build pipeline

Site

Build 20200528.2 failed

Requested by

GitHub

Duration

00:01:58

Build pipeline

Site

[regal arch]

ks129

[python-discord/site] Pull request review submitted: #348 Off\-topic channel names non random selecting

ks129

[python-discord/site] New review comment on pull request #348: Off\-topic channel names non random selecting

This makes that ORM will return queries in random order. Removing this will make that we get always the same names.

[site] Azure Pipelines checks failure on 8eb69db

[ebon magnet]

Build 20200528.3 failed

Requested by

GitHub

Duration

00:01:55

Build pipeline

Site

[north knot]

Postgres backup completed!

[regal arch]

ks129

[python-discord/site] Pull request review submitted: #348 Off\-topic channel names non random selecting

ks129

[python-discord/site] New review comment on pull request #348: Off\-topic channel names non random selecting

This joins these missing names to the original list of names.

[site] Azure Pipelines checks success on 72e3ee0

[ebon magnet]

Build 20200528.4 succeeded

Requested by

GitHub

Duration

00:02:19

Build pipeline

Site

[regal arch]

[site] Azure Pipelines checks success on 9cedba3

[ebon magnet]

Build 20200528.5 succeeded

Requested by

GitHub

Duration

00:02:16

Build pipeline

Site

[regal arch]

lemonsaurus

[bot:redis\_persistence] 1 new commit

b7c30d4 Prevent a state where a coro could wait forever. - lemonsaurus

lemonsaurus

[python-discord/site] Pull request review submitted: #348 Off\-topic channel names non random selecting

lemonsaurus

[python-discord/site] New review comment on pull request #348: Off\-topic channel names non random selecting

ah, nice.

[ebon magnet]

Build 20200528.1 succeeded

Requested by

GitHub

Duration

00:02:24

Build pipeline

Bot

[regal arch]

lemonsaurus

[python-discord/site] Pull request review submitted: #348 Off\-topic channel names non random selecting

These changes seem to have addressed by requests.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

addressed in b7c30d4

[regal arch]

lemonsaurus

[python-discord/django-simple-bulma] Issue opened: #44 Add support for minification

There should be a BULMA_SETTING that would ensure minification of js and css files, when enabled.

lemonsaurus

[python-discord/django-simple-bulma] Issue opened: #45 Use async to defer loading of js, preload CSS\.

When we add the bulma-related JS scripts and the CSS stylesheets into the , we don't enable any kind of deferment or preloading.

This means that sites using django-simple-bulma are taking a performance hit, because these are render-blocking resources.

To fix this, these should be inserted with async and preloading parameters so that they will not block the loading of the page itself.

lemonsaurus

[python-discord/site] New comment on issue #355: Fix issues in https://web\.dev/measure

Here are actual actionable points:

Don't load unused Bulma Extensions

Currently, we're not defining the extensions key in our BULMA_SETTINGS config dict. This means that all Bulma extensions are being loaded, which is stupid because we're not using any of them afaik.

To solve this, just add extensions=[] to BULMA_SETTINGS.

Off-site asset loading has bad caching policies

We're loading a few assets from off-site - the Django logo from djangoproject.com, and some cod...

[regal arch]

SebastiaanZ

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

SebastiaanZ

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

Small comment, this runs before _create_redis_session starts. The previous line just schedules that for execution later in the event loop.

lemonsaurus

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

lemonsaurus

[python-discord/bot] New review comment on pull request #947: RedisCache \- Data Persistence

hm, good point. could move it into the _create_redis_session itself

lemonsaurus

[bot:redis\_persistence] 1 new commit

cc45960 Move the self.redis_closed into session create. - lemonsaurus

[ebon magnet]

Build 20200528.2 succeeded

Requested by

GitHub

Duration

00:02:22

Build pipeline

Bot

[regal arch]

SebastiaanZ

[python-discord/bot] Pull request review submitted: #947 RedisCache \- Data Persistence

SebastiaanZ

[python-discord/bot] Issue closed: #884 Update pyflakes to version 2\.2\.0

SebastiaanZ

[python-discord/bot] Pull request closed: #947 RedisCache \- Data Persistence

SebastiaanZ

[python-discord/bot] branch deleted: redis\_persistence

SebastiaanZ

[bot:master] 80 new commits

3aeb9e6 Adding redis to docker-compose file. - lemonsaurus
5382dd8 Adding redis-py to the Pipfile - lemonsaurus
7a501fd Boilerplate for the RedisCacheMixin - lemonsaurus
588521c Refactor - no more mixins! - lemonsaurus
ee8386e Add basic dict methods for RedisDict. - lemonsaurus

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200528.3 succeeded

Requested by

GitHub

Duration

00:05:10

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

MarkKoz

[bot:bug/filters/928/non\-ascii\-token] 14 new commits

161bf81 Token remover: use regex groups and pass the to... - MarkKoz
bfe79ef Token remover: use finditer instead of findall - MarkKoz
5386eda Token remover: specify Discord epoch in seconds - MarkKoz
4788650 Test token regex won't match non-base64 characters - MarkKoz
e76099d Add more valid tokens to test the regex with - MarkKoz

[regal arch]

aeros

[python-discord/bot] Pull request review submitted: #937 Make token detection more robust and completely rewrite its tests

LGTM. I think the new token inputs are an especially great improvement to the tests. It makes a huge difference in terms of effective test coverage when more realistic inputs are used. Nice work on this, Mark.

[north knot]

Postgres backup completed!

[regal arch]

ks129

[python-discord/bot] Pull request opened: #964 Filtering: Implement bad words detection in nicknames

This uses the same RegEx filter than message bad words filtering. Every alert has 3 days cooldown, storage this to Redis. Uses on_message event. Closes #856

[ebon magnet]

Build 20200529.1 succeeded

Requested by

GitHub

Duration

00:03:03

Build pipeline

Bot

[regal arch]

kwzrd

[python-discord/bot] New comment on pull request #964: Filtering: Implement bad words detection in nicknames

@ Reviewers:

Discussion has taken place in #dev-contrib where we have agreed to use the on_message event. The reasoning is that we do not intend to moderate inactive users. Alert spam / repetition should be prevented by internal logic & caching.

This should explain the discrepancy between @ks129's latest comment on the issue & the implementation as seen in this pull request.

Discord jumplink.

[regal arch]

kwzrd

[python-discord/bot] Pull request review submitted: #964 Filtering: Implement bad words detection in nicknames

I want to share a few initial thoughts on this, but let's wait for input from others before we make any changes.

kwzrd

[python-discord/bot] New review comment on pull request #964: Filtering: Implement bad words detection in nicknames

You can save yourself a call to Redis if you just get the value and abort if it gives None.

Keep in mind that this event will fire for every message sent in the guild, so if we can avoid the superfluous Redis call, we probably should. In fact, it would likely make more sense to poll Redis if and only if a match is found (L153).

In other words, instead of:

1. See if an alert can be sent for given user (regardless of nickname)
2. If so, analyse author name

We can do:

1...

kwzrd

[python-discord/bot] New comment on pull request #964: Filtering: Implement bad words detection in nicknames

Should we consider the use of an asyncio.Lock? The event listen is not atomic and I think a race condition can happen where another event will fire before the first one finishes, resulting in an @everyone ping-spam in #mod-alerts.

[regal arch]

Akarys42

[python-discord/bot] Pull request opened: #965 Ignore response when posting python news

Sometimes a mailing list user doesn't press respond correctly to the email, and so a response is sent as a separate thread. To keep only new threads in the channel, we need to ignore those.

j03b

[python-discord/bot] Pull request review submitted: #965 Ignore response when posting python news

Excellent

[ebon magnet]

Build 20200529.2 succeeded

Requested by

GitHub

Duration

00:02:02

Build pipeline

Bot

[regal arch]

ks129

[python-discord/bot] New comment on pull request #964: Filtering: Implement bad words detection in nicknames

@kwzrd Is this better now?

[ebon magnet]

Build 20200529.3 succeeded

Requested by

GitHub

Duration

00:02:43

Build pipeline

Bot

[regal arch]

SebastiaanZ

[bot:sebastiaan/help\-channels/ratelimits] 2 new commits

63f5028 Fix check_for_answer breaking on missing cache - SebastiaanZ
28f20b9 Reduce the number of help channel name changes - SebastiaanZ

SebastiaanZ

[python-discord/bot] New branch created: sebastiaan/help\-channels/ratelimits

SebastiaanZ

[python-discord/bot] Pull request opened: #966 Remove status emojis from help channels to prevent us from hitting rate limits

Discord has introduced new, strict rate limits for editing the names and topics of individual channels: You may only change the topic/name twice per 10 minutes per channel.

Unfortunately, our help channel system frequently goes over that rate limit as it edits the name and topic of a channel on all three "move" actions we have: to available, to occupied, and to dormant. In addition, our "unanswered" feature adds another channel name change on top of the move-related edits.

That's why I...

j03b

[python-discord/bot] Pull request review submitted: #966 Remove status emojis from help channels to prevent us from hitting rate limits

Looks good to me. If Discord rollback on this we can revert the commit.

[ebon magnet]

Build 20200529.4 succeeded

Requested by

GitHub

Duration

00:02:21

Build pipeline

Bot

[regal arch]

Den4200

[python-discord/bot] Pull request review submitted: #966 Remove status emojis from help channels to prevent us from hitting rate limits

LGTM

j03b

[python-discord/bot] Pull request closed: #966 Remove status emojis from help channels to prevent us from hitting rate limits

j03b

[bot:master] 3 new commits

63f5028 Fix check_for_answer breaking on missing cache - SebastiaanZ
28f20b9 Reduce the number of help channel name changes - SebastiaanZ
dd11ba4 Merge pull request #966 from python-discord/seb... - j03b

j03b

[python-discord/bot] branch deleted: sebastiaan/help\-channels/ratelimits

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200529.5 succeeded

Requested by

Joseph Banks

Duration

00:04:03

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

lemonsaurus

[python-discord/bot] Issue opened: #967 Remove periodic ping in \#verification

This creates "accept bombs" that may trigger our anti-raid and burst spam filters, and is not really necessary anymore. We have more than enough people joining without having to periodically nag at them.

lemonsaurus

[python-discord/bot] Issue opened: #968 \!purge should take an optional set of channels, for mass\-purging multi\-channel spam\.

some people will spam several channels. Let's make all !purge commands default to current channel only, but optionally take a list of channels to purge.

wookie184

[python-discord/bot] New comment on issue #968: \!purge should take an optional set of channels, for mass\-purging multi\-channel spam\.

An option to clear messages from all channels would also be helpful, so if somebody joins just to spam in a ton of channels we can easily clear it without having to specify all channels individually.

MarkKoz

[python-discord/bot] New comment on issue #963: Emoji in help channel name sometimes doesn't change when it becomes occupied

Fixed by #966

MarkKoz

[python-discord/bot] Issue closed: #963 Emoji in help channel name sometimes doesn't change when it becomes occupied

j03b

[python-discord/bot] New comment on issue #968: \!purge should take an optional set of channels, for mass\-purging multi\-channel spam\.

Only issue I foresee with multiple channels is that we need to fetch history from multiple channels or store a cache or the last X messages, though I agree it would be useful.

TeamSpen210

[python-discord/bot] Pull request review submitted: #964 Filtering: Implement bad words detection in nicknames

TeamSpen210

[python-discord/bot] New review comment on pull request #964: Filtering: Implement bad words detection in nicknames

Isn't this itself a race condition (though not much of an impact)? :) Why aren't you creating the lock immediately, if there's a reason it should probably be documented...

MarkKoz

[python-discord/site] Pull request review submitted: #356 No longer accept or track \`avatar\_hash\`

The User model is also created in test_active_infraction_migration.py with an avatar_hash argument. Presumably this is fine since the tests pass. I think it's working with old migrations which use an older model for the User.

MarkKoz

[python-discord/bot] Pull request review submitted: #962 Remove all sending of avatar\_hash

ks129

[python-discord/bot] Pull request review submitted: #964 Filtering: Implement bad words detection in nicknames

ks129

[python-discord/bot] New review comment on pull request #964: Filtering: Implement bad words detection in nicknames

I will add comment tomorrow, but lock is not thread safe.

[regal arch]

lemonsaurus

[python-discord/bot] Issue opened: #969 Staff count for the \!server embed

In Counts, we should have Staff members with the number of Helpers

lemonsaurus

[python-discord/bot] New branch created: staff\_count\_server

MarkKoz

[python-discord/site] New comment on pull request #312: Manage whitelisted guilds and file formats in DB\. \#305

@atmishra what is the status of this?

Den4200

[python-discord/bot] Pull request review submitted: #965 Ignore response when posting python news

Nice.

[ebon magnet]

Build 20200529.6 succeeded

Requested by

GitHub

Duration

00:02:24

Build pipeline

Bot

[regal arch]

Den4200

[python-discord/bot] Pull request closed: #965 Ignore response when posting python news

Den4200

[bot:master] 3 new commits

d12e84f Ignore response when posting python news - Akarys42
7cfbda9 Merge branch 'master' into patch-1 - Den4200
959d7c7 Merge pull request #965 from Akarys42/patch-1 - Den4200

[ebon magnet]

Build 20200529.7 succeeded

Requested by

GitHub

Duration

00:04:10

Build pipeline

Bot

[north knot]

Auto-deployed: bot

[oak estuary]

Python Bot

Connected!

[regal arch]

MarkKoz

[python-discord/bot] New review comment on pull request #864: Ban and kick reason length fix

This is a quite confusing way to do it. Please use a regular if statement rather than inlining it inside an f-string.

MarkKoz

[python-discord/bot] New review comment on pull request #864: Ban and kick reason length fix

This comment seems out of place.

MarkKoz

[python-discord/bot] New review comment on pull request #864: Ban and kick reason length fix

                Actor: {ctx.message.author}{dm_log_text}{expiry_log_text}

MarkKoz

[python-discord/bot] New review comment on pull request #864: Ban and kick reason length fix

This API call may also fail and raise an exception. It should be handled appropriately by including an error message in the mod log and confirmation message (see lines 140-151 to get an idea).

MarkKoz

[python-discord/bot] Pull request review submitted: #864 Ban and kick reason length fix

[regal arch]

MarkKoz

[python-discord/bot] New comment on pull request #927: Truncate internal eval\.

That is an issue on my end but I cannot resolve it. I won't be able to test this PR myself.

[native jolt]

GhostofGoes

[python-discord/flake8-annotations] Pull request review submitted: #83 Release v2\.2\.0

[regal arch]

lemonsaurus

[bot:staff\_count\_server] 2 new commits

aa46d01 Clean up channel counts and add staff channels. - lemonsaurus
171c1e2 Adding staff member count to !server. - lemonsaurus

lemonsaurus

[python-discord/bot] Pull request opened: #970 Add staff counts to \!server

A small change to add number of staff channels and number of staff members to !server. These are nice numbers to track, and may be interesting for everyone.

lemonsaurus

[bot:staff\_count\_server] 4 new commits

d12e84f Ignore response when posting python news - Akarys42
7cfbda9 Merge branch 'master' into patch-1 - Den4200
959d7c7 Merge pull request #965 from Akarys42/patch-1 - Den4200
73e07df Merge branch 'master' into staff_count_server - lemonsaurus

[ebon magnet]

Build 20200529.9 failed

Requested by

GitHub

Duration

00:01:54

Build pipeline

Bot

[regal arch]

MarkKoz

[python-discord/bot] New review comment on pull request #927: Truncate internal eval\.

This should also truncate it to 15 lines.

MarkKoz

[python-discord/bot] Pull request review submitted: #927 Truncate internal eval\.

OK, this all works. Just have one request.

lemonsaurus

[bot:remove\_periodic\_ping] 1 new commit

b258f77 Removing the periodic ping from verification. - lemonsaurus

lemonsaurus

[python-discord/bot] New branch created: remove\_periodic\_ping

SebastiaanZ

[python-discord/bot] Pull request review submitted: #970 Add staff counts to \!server

SebastiaanZ

[python-discord/bot] New review comment on pull request #970: Add staff counts to \!server

This will count #:BIG RED CIRCLE: verification :BIG RED CIRCLE: as a public channel, which it is, I guess; just not for actual members of our guild. I doubt anyone is going to do a manual count and compare the numbers, though.

On a more serious note: One reason this may fail in the future is if we decide to cherry-pick which channels we want to showcase in Discovery. When changing the permissions over, I thought about that, but decided to postpone such a discussion. I think just leaving...

lemonsaurus

[python-discord/bot] Pull request opened: #971 Remove the periodic ping from \#verification

This creates "accept bombs" that may trigger our anti-raid and burst spam filters, and is not really necessary anymore. We have more than enough people joining without having to periodically nag at them.

Closes #967.

lemonsaurus

[bot:remove\_periodic\_ping] 4 new commits

d12e84f Ignore response when posting python news - Akarys42
7cfbda9 Merge branch 'master' into patch-1 - Den4200
959d7c7 Merge pull request #965 from Akarys42/patch-1 - Den4200
650e1b9 Merge branch 'master' into remove_periodic_ping - lemonsaurus

[ebon magnet]

Build 20200529.11 succeeded

Requested by

GitHub

Duration

00:02:17

Build pipeline

Bot

[regal arch]

lemonsaurus

[python-discord/bot] Pull request review submitted: #970 Add staff counts to \!server

lemonsaurus

[python-discord/bot] New review comment on pull request #970: Add staff counts to \!server

This is all true. But the only way to really properly address this would be to check if Helpers, Moderators or Admins had explicit read in these channels, I guess? Even that's not perfect.

MarkKoz

[python-discord/bot] Pull request review submitted: #971 Remove the periodic ping from \#verification

MarkKoz

[python-discord/bot] Pull request review submitted: #970 Add staff counts to \!server

MarkKoz

[python-discord/bot] New review comment on pull request #970: Add staff counts to \!server

How about considering it a staff channel if the roles with read permissions are a subset of the roles with read permissions in the staff lounge? Maybe something like the event channel would get left out though.

[regal arch]

Numerlor

[python-discord/bot] Pull request opened: #972 Specify encoding when reading tag files

Re-adds the change from #875 that got overwritten by #866 when it was marged, causing the bot to crash on windows at startup.

lemonsaurus

[python-discord/bot] Pull request review submitted: #972 Specify encoding when reading tag files

[ebon magnet]

Build 20200529.12 succeeded

Requested by

GitHub

Duration

00:02:35

Build pipeline

Bot

[regal arch]

MarkKoz

[python-discord/bot] Pull request review submitted: #972 Specify encoding when reading tag files

MarkKoz

[python-discord/bot] Pull request closed: #972 Specify encoding when reading tag files

MarkKoz

[bot:master] 2 new commits

f7fe7df Tags: explicitly use UTF-8 to read files - MarkKoz
0878293 Merge pull request #972 from Numerlor/tag-encoding - MarkKoz

[north knot]

Auto-deployed: bot

[ebon magnet]

Build 20200529.13 succeeded

Requested by

GitHub

Duration

00:04:00

Build pipeline

Bot

[oak estuary]

Python Bot

Connected!

[regal arch]

lemonsaurus

[bot:staff\_count\_server] 2 new commits

89752c5 More precise staff-channel check. - lemonsaurus
f59e634 Fix busted test_information test. - lemonsaurus

[ebon magnet]

Build 20200529.14 succeeded

Requested by

GitHub

Duration

00:02:07

Build pipeline

Bot

[ebon magnet]

Build 20200530.1 succeeded

Requested by

GitHub

Duration

00:02:27

Build pipeline

Bot

Build 20200530.2 succeeded

Requested by

GitHub

Duration

00:02:18

Build pipeline

Bot

[north knot]

Postgres backup completed!