neoliberal / user_pinger_2

Bot + accessory services for pinging groups on a subreddit
MIT License
9 stars 2 forks source link

Logging for API #2

Open theredcameron opened 2 years ago

theredcameron commented 2 years ago

There's a comment in the api.py file that specifies that logging needs to be added. How did you picture the logging to be accomplished? Do you intent to log to Slack, SQLite database, or a log file?

jenbanim commented 2 years ago

I'd like to integrate this with the slack_python_logging module. Check out api.py to see how this logging system is used

The logging issue is tied in with a similar issue I have with testing api.py. What I'd like to do is mock the Reddit instance, run Uvicorn, and test the API endpoints with requests-unixsocket. Currently the Reddit state just lives in the global namespace of api.py so it's not possible to do that

I think it might make sense to put all the api.py stuff into a class and call a run function similar to how bot.py does it currently. By putting the state into a class, we could pass the Reddit and (possibly) logger instances as arguments to instantiating the api object

theredcameron commented 2 years ago

I like the idea of putting it all into a class like in bot.py. Also, I think we should make it so that rather than the bot.py an api.py files interfacing directly with the reddit and Slack libraries, we should put the slack and reddit implementations into their own 'services' or files that abstract the reddit specific stuff out so that we can more cleanly access those library's methods from bot.py and api.py.

Also, if we use an interface to define how the bot.py and api.py files use these 'services', we can easily create mock classes for testing so we can mock out the reddit and slack libraries when running automated tests.

This implementation would also have the advantage of making it easier to make adjustments if Slack or reddit change the way we need to call their API. We could change the services only and leave files like bot.py and api.py untouched.

theredcameron commented 2 years ago

@jenbanim Let me know your thoughts.