meraki-analytics / cassiopeia-datastores

MIT License
3 stars 6 forks source link

"accountId" variable type is defined too small #7

Closed m3adow closed 6 years ago

m3adow commented 6 years ago

I'm using the cassiopeia-sqlstore plugin with PostgreSQL as database.

The "accountId" in the "summoner" table is only defined as an integer, resulting in 2147483647 being the biggest possible number in PostgreSQL. But there are accounts with way higher Ids, e.g. summoner "giverank1" with the summoner Id "20182777" in Turkey (TR) has the accountId 1946032892047936.
As of now, I've encountered this error in Turkey and Japan, so perhaps it's limited to newer regions.

If I'm not mistaken, the same problem could apply to "p_accountId" in the "match_participant_identities" table.

Sadly, I'm not very experienced with SQLAlchemy, so I don't think I'm able to submit a PR. I suspect changing the type to a bigint should be sufficient, but I'm very uncertain.

Here is the full resulting stack trace (with anonymized pathes):

Exception in thread T-TR-0:
Traceback (most recent call last):
  File "C:\virtualenv\lib\site-packages\merakicommons\ghost.py", line 41, in wrapper
    return method(*args, **kwargs)
  File "C:\virtualenv\lib\site-packages\cassiopeia\core\summoner.py", line 160, in id
    return self._data[SummonerData].id
AttributeError: 'SummonerData' object has no attribute 'id'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\virtualenv\lib\site-packages\merakicommons\ghost.py", line 87, in __get__
    return self.fget(obj)
  File "C:\virtualenv\lib\site-packages\merakicommons\ghost.py", line 43, in wrapper
    raise GhostLoadingRequiredError(str(error))
merakicommons.ghost.GhostLoadingRequiredError: 'SummonerData' object has no attribute 'id'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\virtualenv\lib\site-packages\sqlalchemy\engine\base.py", line 1193, in _execute_context
    context)
  File "C:\virtualenv\lib\site-packages\sqlalchemy\engine\default.py", line 508, in do_execute
    cursor.execute(statement, parameters)
psycopg2.DataError: integer out of range

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "C:\python\lib\threading.py", line 916, in _bootstrap_inner
    self.run()
  File "C:\python\lib\threading.py", line 864, in run
    self._target(*self._args, **self._kwargs)
  File "C:\app\app\update_league_games.py", line 60, in get_league_games
    live_game = cass.get_current_match(entry.to_dict()['summonerName'], league.region)
  File "C:\virtualenv\lib\site-packages\cassiopeia\cassiopeia.py", line 76, in get_current_match
    return CurrentMatch(summoner=summoner, region=region)
  File "C:\virtualenv\lib\site-packages\cassiopeia\core\common.py", line 211, in __call__
    query = cls.__get_query_from_kwargs__(**kwargs)
  File "C:\virtualenv\lib\site-packages\cassiopeia\core\common.py", line 49, in default_region_wrapper
    return method(*args, **kwargs)
  File "C:\virtualenv\lib\site-packages\cassiopeia\core\spectator.py", line 207, in __get_query_from_kwargs__
    query["summoner.id"] = Summoner(name=summoner, region=region).id
  File "C:\virtualenv\lib\site-packages\merakicommons\ghost.py", line 90, in __get__
    obj.__load__(load_group)
  File "C:\virtualenv\lib\site-packages\cassiopeia\core\common.py", line 277, in __load__
    data = configuration.settings.pipeline.get(type=self._load_types[load_group], query=query)
  File "C:\virtualenv\lib\site-packages\datapipelines\pipelines.py", line 459, in get
    return handler.get(query, context)
  File "C:\virtualenv\lib\site-packages\datapipelines\pipelines.py", line 190, in get
    sink.put(result, context)
  File "C:\virtualenv\lib\site-packages\datapipelines\pipelines.py", line 140, in put
    self._sink.put(self._store_type, item, context)
  File "C:\virtualenv\lib\site-packages\datapipelines\sinks.py", line 62, in wrapper
    return call(self, items, context=context)
  File "c:\virtualenv\src\cassiopeia-sqlstore\cassiopeia-sqlstore\cassiopeia_sqlstore\SQLStore.py", line 196, in put_summoner
    self._put(SQLSummoner(**item))
  File "c:\virtualenv\src\cassiopeia-sqlstore\cassiopeia-sqlstore\cassiopeia_sqlstore\SQLStore.py", line 98, in inner
    session.commit()
  File "C:\virtualenv\lib\site-packages\sqlalchemy\orm\session.py", line 943, in commit
    self.transaction.commit()
  File "C:\virtualenv\lib\site-packages\sqlalchemy\orm\session.py", line 467, in commit
    self._prepare_impl()
  File "C:\virtualenv\lib\site-packages\sqlalchemy\orm\session.py", line 447, in _prepare_impl
    self.session.flush()
  File "C:\virtualenv\lib\site-packages\sqlalchemy\orm\session.py", line 2254, in flush
    self._flush(objects)
  File "C:\virtualenv\lib\site-packages\sqlalchemy\orm\session.py", line 2380, in _flush
    transaction.rollback(_capture_exception=True)
  File "C:\virtualenv\lib\site-packages\sqlalchemy\util\langhelpers.py", line 66, in __exit__
    compat.reraise(exc_type, exc_value, exc_tb)
  File "C:\virtualenv\lib\site-packages\sqlalchemy\util\compat.py", line 187, in reraise
    raise value
  File "C:\virtualenv\lib\site-packages\sqlalchemy\orm\session.py", line 2344, in _flush
    flush_context.execute()
  File "C:\virtualenv\lib\site-packages\sqlalchemy\orm\unitofwork.py", line 391, in execute
    rec.execute(self)
  File "C:\virtualenv\lib\site-packages\sqlalchemy\orm\unitofwork.py", line 556, in execute
    uow
  File "C:\virtualenv\lib\site-packages\sqlalchemy\orm\persistence.py", line 181, in save_obj
    mapper, table, insert)
  File "C:\virtualenv\lib\site-packages\sqlalchemy\orm\persistence.py", line 830, in _emit_insert_statements
    execute(statement, multiparams)
  File "C:\virtualenv\lib\site-packages\sqlalchemy\engine\base.py", line 948, in execute
    return meth(self, multiparams, params)
  File "C:\virtualenv\lib\site-packages\sqlalchemy\sql\elements.py", line 269, in _execute_on_connection
    return connection._execute_clauseelement(self, multiparams, params)
  File "C:\virtualenv\lib\site-packages\sqlalchemy\engine\base.py", line 1060, in _execute_clauseelement
    compiled_sql, distilled_params
  File "C:\virtualenv\lib\site-packages\sqlalchemy\engine\base.py", line 1200, in _execute_context
    context)
  File "C:\virtualenv\lib\site-packages\sqlalchemy\engine\base.py", line 1413, in _handle_dbapi_exception
    exc_info
  File "C:\virtualenv\lib\site-packages\sqlalchemy\util\compat.py", line 203, in raise_from_cause
    reraise(type(exception), exception, tb=exc_tb, cause=cause)
  File "C:\virtualenv\lib\site-packages\sqlalchemy\util\compat.py", line 186, in reraise
    raise value.with_traceback(tb)
  File "C:\virtualenv\lib\site-packages\sqlalchemy\engine\base.py", line 1193, in _execute_context
    context)
  File "C:\virtualenv\lib\site-packages\sqlalchemy\engine\default.py", line 508, in do_execute
    cursor.execute(statement, parameters)
sqlalchemy.exc.DataError: (psycopg2.DataError) integer out of range
 [SQL: 'INSERT INTO summoner (platform, id, "accountId", name, "summonerLevel", "profileIconId", "revisionDate", "lastUpdate") VALUES (%(platform)s, %(id)s, %(accountId)s, %(name)s, %(summonerLevel)s, %(profileIconId)s, %(revisionDate)s, %(lastUpdate)s)'] [parameters: {'platform': 'TR1', 'id': 20182777, 'accountId': 1946032892047936, 'name': 'giverank1', 'summonerLevel': 38, 'profileIconId': 7, 'revisionDate': 1528202508000, 'lastUpdate': 1528209198.063921}] (Background on this error at: http://sqlalche.me/e/9h9h)
Satrium commented 6 years ago

Will give it a look tomorrow, when I'm back at my work station. Should be a quick fix. Do you know if summonerIDs are affected as well? I only know that matchIds are using BigIntegers because they exceed 32bit.

m3adow commented 6 years ago

I just now had time to check for summonerIDs. I checked all Challenger and Master accounts in every region and couldn't find any occasion of a summonerId being too big for a normal int. So I suppose the problem does not apply to summonerIDs.

Satrium commented 6 years ago

Thanks for checking summoner ids. I've fixed the bug in PR #8. You can pull it from github as soon as it gets merged:
pip install -e 'git+https://github.com/meraki-analytics/cassiopeia-datastores#egg=cassiopeia-sqlstore&subdirectory=cassiopeia-sqlstore' As I wrote in the PR message, the fix will only be in affect after the database scheme has been changed. You can do that manually (if possible in your database) or delete affected tables and let the library rebuild them.

jjmaldonis commented 6 years ago

Ok just merged, thanks @Satrium for always being on top of this even when I'm not!