Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding wolfram group of commands to gurkbot #106

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

dhzdhd
Copy link

@dhzdhd dhzdhd commented Mar 12, 2021

PR related to the issue #53

Things added:
1) A wolfram cog in the fun folder in cogs
2) A wolfram group of commands which , for now, consists of the image command
Things changed:
1) wolfram_emoji added to constants.py
2) Tokens class for the app_id for the Wolfram API

image command:
> Fetches an image url based on a user query
> This url is embedded and sent to the respective channel it was called in

Note:
The wolfram API requires a token which has to be provided for in the .env as:
WOLFRAM_TOKEN="<token_here>"

Copy link
Contributor

@Shivansh-007 Shivansh-007 left a comment

Choose a reason for hiding this comment

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

Quick first review

bot/constants.py Outdated Show resolved Hide resolved
bot/exts/fun/wolfram.py Outdated Show resolved Hide resolved
bot/exts/fun/wolfram.py Outdated Show resolved Hide resolved
bot/exts/fun/wolfram.py Outdated Show resolved Hide resolved
bot/exts/fun/wolfram.py Outdated Show resolved Hide resolved
bot/exts/fun/wolfram.py Outdated Show resolved Hide resolved
@dhzdhd dhzdhd marked this pull request as ready for review January 6, 2022 14:23
Copy link
Contributor

@ShakyaMajumdar ShakyaMajumdar left a comment

Choose a reason for hiding this comment

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

LGTM

@onerandomusername onerandomusername linked an issue Jan 8, 2022 that may be closed by this pull request
Copy link
Contributor

@onerandomusername onerandomusername left a comment

Choose a reason for hiding this comment

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

Welcome, and thank you for your first pull to gurkbot!

Given how the wolfram api works, I have a few requests. I would also like to point out the app_id is the secret, so we shouldn't be using our request as the image link.

Additionally, a user can expose the hosting provider's location, IP, and other information by searching 'Where am I'.

To mitigate these impacts, we can provide an ip and location with query parameters.
image

On the good side of things, thank you for putting the token in .env-example, and not once did I receive an error saying the application timed out or did not reply.

bot/exts/fun/wolfram.py Outdated Show resolved Hide resolved
bot/exts/fun/wolfram.py Outdated Show resolved Hide resolved
bot/exts/fun/wolfram.py Show resolved Hide resolved
@onerandomusername onerandomusername added area: commands Commands, cogs and extensions status: needs review type: feature A request for or implementation of a new feature labels Jan 14, 2022
Copy link
Member

@Akarys42 Akarys42 left a comment

Choose a reason for hiding this comment

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

Blocking this while we don't have the API key set up in prod

@Akarys42 Akarys42 dismissed their stale review March 25, 2022 16:06

API key up and running!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: commands Commands, cogs and extensions status: needs review type: feature A request for or implementation of a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Wolfram command for gurkbot
6 participants