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

Match version does not match static data version #281

Open
xEcEz opened this issue Apr 4, 2019 · 2 comments
Open

Match version does not match static data version #281

xEcEz opened this issue Apr 4, 2019 · 2 comments

Comments

@xEcEz
Copy link
Contributor

xEcEz commented Apr 4, 2019

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
Copy link
Contributor Author

xEcEz commented Apr 7, 2019

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
Copy link
Member

robrua commented Apr 9, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants