Skip to content

Run null check on needed API keys before loading cogs.#1269

Merged
wookie184 merged 5 commits into
python-discord:mainfrom
shtlrs:load-cogs-when-config-is-setup
May 6, 2023
Merged

Run null check on needed API keys before loading cogs.#1269
wookie184 merged 5 commits into
python-discord:mainfrom
shtlrs:load-cogs-when-config-is-setup

Conversation

@shtlrs
Copy link
Copy Markdown
Contributor

@shtlrs shtlrs commented May 6, 2023

We have a lot of Cogs that need some API key to function.

If the key isn't present, it would result in constant 401s.

This PR adds a change that won't load these Cogs & log a warning whenever these keys aren't present.

This makes sure that all cogs that need keys won't load.

Which helps in avoiding exceptions at runtime, and give a clearer warning at startup time.
@shtlrs shtlrs requested a review from ks129 as a code owner May 6, 2023 13:08
@shtlrs shtlrs changed the title Run null check on needed api keys before loading cogs. Run null check on needed API keys before loading cogs. May 6, 2023
Copy link
Copy Markdown
Contributor

@wookie184 wookie184 left a comment

Choose a reason for hiding this comment

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

There's currently an error log for bot.exts.utilities.reddit when missing the token, could we make that a warning for consistency?

Comment thread bot/exts/fun/snakes/__init__.py Outdated
Comment on lines +12 to +13
if not Tokens.giphy:
log.warning("No Youtube token. All youtube related commands in Snakes cog won't work.")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like the wrong constant? Also capitalise youtube as YouTube

Copy link
Copy Markdown
Contributor Author

@shtlrs shtlrs May 6, 2023

Choose a reason for hiding this comment

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

Caught red-handed 5bdd195 👀

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There's currently an error log for bot.exts.utilities.reddit when missing the token, could we make that a warning for consistency?

Yessir we can 0e202c0

Comment thread bot/exts/utilities/wolfram.py Outdated

async def setup(bot: Bot) -> None:
"""Load the Wolfram cog."""
if not Wolfram.key:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wolfram is the cog, not the constants config. This causes an error.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

My bad, I hadn't still tested this yet, and I was trying to get something in a PR before I left the house.
5ba5275

@shtlrs shtlrs requested a review from wookie184 May 6, 2023 16:02
Copy link
Copy Markdown
Contributor

@wookie184 wookie184 left a comment

Choose a reason for hiding this comment

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

looks good, I missed one thing on my previous review.

Comment thread bot/exts/holidays/halloween/spookygif.py Outdated
Co-authored-by: wookie184 <wookie1840@gmail.com>
Copy link
Copy Markdown
Contributor

@wookie184 wookie184 left a comment

Choose a reason for hiding this comment

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

This is a nice QOL improvement, thanks!

@wookie184 wookie184 merged commit 56cefe8 into python-discord:main May 6, 2023
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.

3 participants