start-again / spookyBot

🎃 A spooky Discord bot
MIT License
9 stars 12 forks source link

Feature/spooktober picture command #48

Open matteoauger opened 3 years ago

matteoauger commented 3 years ago

Aims to close #47

What have been implemented

I added a command which displays a random picture from r/spooktober on the discord server. Command name is the following : boo!meme.

Example here : example picture

How it's been implemented

I created a new command in the commands/Picture directory. This command uses axios module to asynchronously send requests to reddit API. As long as the command only reads data from the API, no token or other auth is required for it to work.

I hope you'll like it :+1:

tmttn commented 3 years ago

EDIT: but I see now that your code does not use the Reddit API, unlike you stated in the PR description. So this is basically scraping the subreddit. But using .jsonisn't even allowed either way, not even authenticated, looking at https://www.reddit.com/robots.txt...

User-Agent: *
Disallow: /*.json
Disallow: /*.json-compact
Disallow: /*.json-html

Also another passage of the API rules:

We're happy to have API clients, crawlers, scrapers, and browser extensions, but they have to obey some rules: Please ensure that all API clients follow Reddit's API terms Clients must authenticate with OAuth2 Directly from the Reddit API terms: All API clients must authenticate with OAUTH 2

That being said, I have implemented an authenticated implementation in my PR, you might pull some inspiration from that for this one. We might have to implement some rate limiting / caching towards the Reddit API though...


Reddit API Access We want to allow developers to build great products powered by Reddit and we recognize our developer community is integral to the success of the Reddit platform. We also want protect our users’ privacy and security regardless of how they choose to consume Reddit content.

In order to access the Reddit API directly, please make sure you comply with the following steps:

You must read the terms and register in order to use the Reddit API All API clients must authenticate with OAUTH 2 All API clients must follow the API rules: https://github.com/reddit/reddit/wiki/API You may not use the Reddit logos and trademark without approval from us You may use "for reddit" or "a client for reddit" in the title of your app. You may not use "reddit" without "for" preceding it. You may not use the word "official" in the title, keywords or description in any way that implies the app was developed by Reddit, Inc. If your intended usage is commercial, you’ll need approval from us (either by filling out the API terms form or emailing api@reddit.com. Use of the API is considered "commercial" if you are earning money from it, including, but not limited to in-app advertising, in-app purchases or you intend to learn from the data and repackage for sale. Open source use is generally considered non-commercial.

matteoauger commented 3 years ago

Thanks I will look at it :)

matteoauger commented 3 years ago

@tmttn Hello again, I worked on the command to properly use Reddit API like you did (I used the same config format and module as your PR to avoid conflicts 👍 ) I didn't mind the robots.txt. To me it was relevant for webcrawlers only, but thanks to you I've learn something important 🙏

About the API usage, I couldn't figure out how to filter image posts properly, so I did it my way based on whether or not the url contains https://i.redd.it. I think this is not a good way to do it, if you know a better way fell free to tell me.

Finally, I don't really know how to avoid reaching the 60 requests limit for now.

illusi0nary commented 3 years ago

We need to prepare the reddit API in beforehand (server sided) so we can actually use it. :-)

tmttn commented 3 years ago

I have also discussed #42 and this one with @LucasCtrl and we came to the conclusion that in order to make these Reddit commands work, we'll have to decouple actual command executions from interacting with the Reddit API. This is required because otherwise we will hit rate limits fast when more users start using the bot (because the API limit is shared between all users).

My proposal is to cache the results we need, and call the API x times per hour to update this cache (or even x times per day). After having cached a number of results, we display a random one of those results.

tmttn commented 3 years ago

@matteoauger something looks off here, why are there so many commits visible in the PR? It looks like you didn't use git merge upstream/main to merge changes from the main upstream branch into your PR branch.

matteoauger commented 3 years ago

My bad, it's actually my first time resolving a PR using command line only. I used git rebase upstream/main instead. I'll use git merge upstream/main next times.