meraki-analytics / cassiopeia

An all-inclusive Python framework for the Riot Games League of Legends API. Cass focuses on making the data easy and fun to work with, while providing all the tools necessary to create a website or do data analysis.
MIT License
553 stars 134 forks source link

Match version does not match static data version #281

Open xEcEz opened 5 years ago

xEcEz commented 5 years ago

The current match version 9.7.269.2391 (mapped to to 9.7.1 in cass) is used to load any item related to that match, causing errors since the static files it refers to does not exist.

>>> m = Match(id=3984111553, region='EUW')
Making call: https://euw1.api.riotgames.com/lol/match/v4/matches/3984111553
>>> m.version
'9.7.269.2391'
>>> p = m.participants[0]
>>> p.champion.id
13
>>> p.champion.name
Making call: https://ddragon.leagueoflegends.com/cdn/9.7.1/data/en_GB/championFull.json
Traceback (most recent call last):
  File "/home/xec/dev/wezzar/venv/lib/python3.6/site-packages/merakicommons/ghost.py", line 41, in wrapper
    return method(*args, **kwargs)
  File "/home/xec/dev/wezzar/scripts/cassiopeia/core/staticdata/champion.py", line 704, in name
    return self._data[ChampionData].name
AttributeError: 'ChampionData' object has no attribute 'name'

I've hardcoded something on my end to force the match version to be set to 9.6.1 when it is set to 9.7.1 in cass, but that's obviously not the way to go. I am curious to hear why the match version is already set to 9.7.X, but I guess it's just Riot messing with the IDs again.

Maybe cass should check wether the call to the static data returned the expected object and if it is not the case, use a previous version to actually fetch it.

xEcEz commented 5 years ago

It's fixed for now since Riot released static data for 9.7.1, but I guess implementing some kind of fallback mechanism is still a good idea.

robrua commented 5 years ago

Yeah this was really unfortunate. I've been thinking about what cass' behavior in this sort of situation should be and while I like the idea of having a fallback, I'm wary of a couple of things:

1) I can imagine some folks who may be using cass for theorycrafting or the like and wouldn't want cass automatically downgrading the version of the items it's pulling so they don't reflect what the match actually used. In these cases, we could print a logger warning that notifies the user, but I think for that case an exception would be ideal. Possibly making the behavior configurable is the answer here (with the default being "just show a warning", I think) but I'd like to get Jason's perspective once he's back from his trip and hash out what makes the most sense.

2) I'm wondering whether this sort of fallback makes sense as a cass feature, or whether it should be delegated to the user (possibly with some tools to help make the handling easier). We ride the line a lot between "just getting you Riot data" and "being part of your backend" when it comes to datastores and the like. I think something like this though, which feels like we're starting to get concerned with preserving the downstream application's uptime, is a small step in a direction we haven't gone before. I want to think a little about whether that direction makes sense to follow.

Those things said, this is also something that is (hopefully) sufficiently infrequent that we won't see it many more times in the future. I'd like to get Jason to weigh in on these concerns before we make a change, as it's probably going to require the addition of some new message passing between core and the datapipeline to support this.

Let me know if you have any other thoughts in the meantime and thanks for bringing this up, Rob