zacholade / Rift-Companion

1 stars 0 forks source link

Implementing an async IO #11

Open zacholade opened 5 years ago

zacholade commented 5 years ago

Had a conversation with the repo maintainer of casseopeia on discord ( https://discord.gg/mTwdsDN ).

Jason (Kalturi)Yesterday at 20:19
@Ghostal 

Ok so you clearly have worked with this quite a bit. I'm going to give my thoughts... You mentioned the ghost loading, and that's closely linked with the really nice class system that Cass implements, which allows you do great stuff like match.participants[Summoner("Kalturi")].champion.name. However, that type system and the underlying functionality to make it fun/easy to use is really complex. On the other hand, the functionality to access the data that Riot gives in dictionary/json format is quite simple, and that's the data format that Riot-Watcher and Pantheon (canisback) both provide. It's not as fun or easy to work with, but it's much much easier to get async (in python) working with that. Frankly, Python's async stuff imo doesn't have a great interface, and it's kinda hard to work with. You've certainly already felt some of that. That means that getting asyncio to work with Cass's type/class system is going to be really difficult and it's pretty much going to require completely new code and a new thinking process to understand how the async stuff is going to fit together.
In addition to the complex type/class system that Cass implements, the datapipeline is also somewhat complicated but that's mostly because of all the pieces that can be connected to the pipeline. Once you introduce async, you have to start worrying about thread safety in all your database connections, and every other data store that's plugged into the datapipeline. That's something that we haven't put much thought into, so it's unlikely that the current functionality in Cass + the datapipeline is threadsafe. That means you'll likely run into some difficult to debug issues unless you rewrite those integrations from scratch with threadsafety in mind.
Honestly I don't think it's worth spending the time to get async working with Cass. I say that partly because its sister library Orianna (java) has much better thread safety -- mostly because java is just much better at thread safety and async than python is. If I were you I think I'd go one of two routes: 1) use Orianna as much as possible and maybe rewrite your discord app in java entirely, or 2) pull some of our datapipeline integrations out of Cass and integrate them with Pantheon and with Canisback's staticdata repo (assuming that's also async).
I realize that's probably not at all what you wanted to hear... but on the other hand I don't want you to spend a ton of time getting async to work with Cass unless you really want to
GhostalYesterday at 20:37
@Jason (Kalturi) Really appreciate the detailed response and reasoning - thanks.
Ghost loading definitely has huge benefits as you pointed out and is definitely not something which should be removed at a cost of implementing async I definitely agree.

Would love to explore Orianna however would be at a loss of a few years of experience which I have in python which i dont in java.
This leaves my options to either
a) continue using executors to execute the blocking code and just work awkwardly around the problem present
b) or make a 'fork' pantheon's base idea of using async requests and with combine minimal parts of cass's data pipeline (exclusive to storing it in an sql table pretty much)
When I got time I may look into option b because a doesn't really resolve the issue
:joy:

Long story short. It is near impossible to implement an async IO in cass. There are other options such as pantheon https://github.com/Canisback/pantheon/blob/master/pantheon/pantheon.py which is async (but personally I think I can do a better job).

I'm therefore making a public asynchronous framework for the riot and championGG api which can be followed here. https://github.com/Zachy24/poro

This issue will remain open until this IO is added.