Skip to content

Add AoC task functionality#45

Merged
mbaruh merged 48 commits into
python-discord:mainfrom
Robin5605:off-season-aoc
Aug 9, 2022
Merged

Add AoC task functionality#45
mbaruh merged 48 commits into
python-discord:mainfrom
Robin5605:off-season-aoc

Conversation

@Robin5605
Copy link
Copy Markdown
Contributor

@Robin5605 Robin5605 commented Jun 3, 2022

Closes #44

This is meant to be a proof of concept, and get the basic functionality working. There are a few things that could be improved upon (better error messages, use embeds instead of regular messages for info messages, better info messages, etc), and some features such as restricting the command to certain users/roles (admins/owners) have been left out for the time being, as I do not have enough information to code that up. However, that should be relatively easy. I welcome any and all suggestions to improve upon this, especially in the "design" department with error messages and embeds for info messages.

@Robin5605
Copy link
Copy Markdown
Contributor Author

Should now reflect this comment

Comment thread bot/exts/aoc.py Outdated
Comment thread bot/constants.py Outdated
Comment thread bot/exts/aoc.py Outdated
Comment thread bot/exts/aoc.py Outdated
Comment thread bot/exts/aoc.py Outdated
Comment thread bot/exts/aoc.py Outdated
Comment thread bot/exts/aoc.py Outdated
@Robin5605 Robin5605 requested a review from camcaswell June 26, 2022 23:06
@camcaswell camcaswell requested a review from janine9vn July 8, 2022 01:43
camcaswell
camcaswell previously approved these changes Jul 8, 2022
@camcaswell
Copy link
Copy Markdown
Contributor

The test is failing intentionally at the moment because there's a placeholder for a thread that doesn't exist yet. You can test locally by simply defining it in your .env, and I'll commit the thread ID when we create it.

@minalike minalike dismissed camcaswell’s stale review July 24, 2022 19:43

Contributed to the PR

Copy link
Copy Markdown
Contributor

@minalike minalike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a couple suggested changes but no blockers.

Tested the following things:

  • Starting and stopping event
  • Permission role lock
  • Force-posting a question
  • Setting a custom scheduled post-time other than UTC midnight (it posted!)
screenshots image image

Forced post
image

Scheduled post at 3AM UTC (11PM my time)
image

image
logs if you are interested
sir-robin              | 2022-08-02 02:46:52,538 | async_rediscache.types.base | DEBUG | Released <NamespaceLock namespace='sir-robin.SummerAoC.cache' [unlocked]> for RedisCache.update
sir-robin              | 2022-08-02 02:46:52,701 | bot.exts.summer_aoc | INFO | Starting Summer AoC event with self.year=2017 self.current_day=7 self.day_interval=1
sir-robin              | 2022-08-02 02:46:52,704 | bot.exts.summer_aoc | DEBUG | Waiting until 3:00 UTC to start Summer AoC loop
sir-robin              | 2022-08-02 03:00:00,790 | bot.exts.summer_aoc | INFO | Posting puzzle for day 7
sir-robin              | 2022-08-02 03:00:01,809 | async_rediscache.types.base | DEBUG | Trying to acquire <NamespaceLock namespace='sir-robin.SummerAoC.cache' [unlocked]> for RedisCache.update
sir-robin              | 2022-08-02 03:00:01,811 | async_rediscache.types.base | DEBUG | Acquired <NamespaceLock namespace='sir-robin.SummerAoC.cache' [locked]> for RedisCache.update
sir-robin              | 2022-08-02 03:00:01,814 | async_rediscache.types.cache | DEBUG | Updating the cache with the following items:
sir-robin              | {'is_running': True, 'year': 2017, 'current_day': 8, 'day_interval': 1, 'post_time': 3}
sir-robin              | 2022-08-02 03:00:01,824 | async_rediscache.types.base | DEBUG | Released <NamespaceLock namespace='sir-robin.SummerAoC.cache' [unlocked]> for RedisCache.update

If there are suggestions for other things I should test, let me know. I plan on testing how it fares with a bot restart.

Comment thread bot/exts/summer_aoc.py Outdated
Comment thread bot/exts/summer_aoc.py
Comment thread bot/exts/summer_aoc.py
camcaswell and others added 2 commits August 2, 2022 13:37
Co-authored-by: mina <75038675+minalike@users.noreply.github.com>
Co-authored-by: mina <75038675+minalike@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@janine9vn janine9vn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two big things:

  1. Required change in pyproject.toml so this doesn't break locally and we can test it. We'll have to take a look into our redis cache code to adopt to the the async_rediscache breaking changes when it was migrated to aioredis v2.

  2. There's a bug I can't quite hunt down but it might be in utils/next_time_occurrence(), but if I start RoC there's an issue with the timestamp generation for when the next puzzle will be posted. It'll say the next puzzle will be posted at the exact time that message was posted. This doesn't happen when I force a post, but as far as I can tell it does when it's regularly scheduled.

I don't think it's an issue with naive timestamps and running in non-utc since arrow does non-naive by default. I'm going to let my bot run for the next 24 hours to see if it's an issue with any scheduled post or just for the first one.

image

The rest of the comments are more run-of-the-mill or musings that can be ignored or deferred. As a general note, it might be worth prefixing a lot of the internal functions with _ just to maintain code style cohesion with the rest of our code base (thinking Lance and Bot). But that's also very minor and very much up for debate.

Comment thread pyproject.toml Outdated
Comment thread bot/exts/summer_aoc.py Outdated
Comment thread bot/exts/summer_aoc.py
Comment thread bot/exts/summer_aoc.py
camcaswell and others added 3 commits August 6, 2022 20:14
Co-authored-by: Janine vN <janine9vn@gmail.com>
* Store the date of the first post as a reference to calculate which days should be posted on in the future.
* Use that date to properly calculate the time of the next post.
* Include start date in info embed
Copy link
Copy Markdown
Contributor

@janine9vn janine9vn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All my blocker comments are resolved and it works nicely~

@mbaruh mbaruh merged commit 824d571 into python-discord:main Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Off season AoC command

5 participants