cesium-ml / baselayer

Fully customizable (scientific, compute-intensive) web application template
http://cesium-ml.org/baselayer/
31 stars 18 forks source link

Error with the new config method cfg.get(key, default) #320

Closed Theodlz closed 2 years ago

Theodlz commented 2 years ago

@stefanv when I pin the latest version of baselayer in SkyPortal, I get:

File "/home/theodlz/shift_management_admin_menu/skyportal/skyportal/facility_apis/__init__.py", line 2, in <module>
[12:16:36 migration_manager]     from .atlas import ATLASAPI
[12:16:36 migration_manager]   File "/home/theodlz/shift_management_admin_menu/skyportal/skyportal/facility_apis/atlas.py", line 20, in <module>
[12:16:36 migration_manager]     if cfg['app.atlas.port'] is None:
[12:16:36 migration_manager]   File "/home/theodlz/shift_management_admin_menu/skyportal/baselayer/app/config.py", line 67, in __getitem__
[12:16:36 migration_manager]     val = dict.__getitem__(val, key)
[12:16:36 migration_manager] KeyError: 'port'
Theodlz commented 2 years ago

What is missing is a try catch in case the key doesn't exist

Theodlz commented 2 years ago

@stefanv something like:

try:
    val = dict.__getitem__(val, key)
except KeyError:
    raise KeyError(f"Key {key} not found in {val}")
Theodlz commented 2 years ago

But that still raises an error, and the app cant start. Just log might be better ?

Theodlz commented 2 years ago

Or, maybe it is a good thing that it raises an error, and it tells us that the key that causes this error should be added to SkyPortal's config. In the config, atlas and kait are missing a port

stefanv commented 2 years ago

Yes, I think if you request a key that's not in the config that's the correct error. Wherever that's called, if they want the None by default, now get is the way to make that explicit.

Theodlz commented 2 years ago

Should I just open a PR on skyportal to use the new syntax everywhere ? And to correct the missing keys in the config ?

stefanv commented 2 years ago

Are we using missing keys in many places?

Theodlz commented 2 years ago

I'm not sure. I get an error related to a key and the app breaks and doesnt start I'm gonna open a PR for this now.

Theodlz commented 2 years ago

PR is ready I'm about to open it. Turns out that besides the missing ports in SkyPortal's config. There are so mistakes in that app_server with incorrect keys it seems. I changed them and it works now. For example, a part checks if the app's secret_key is still abc01234 and tells you that the app is not secure. Well, that never showed up for me, and that's why. The key seems to be incorrect.

Theodlz commented 2 years ago

Update: Seems like pinning the latest version of baselayer introduces errors with SQL alchemy. Here is what I get when accessing the app.

ERROR:tornado.application:Uncaught exception GET /api/groups?includeSingleUserGroups=true (127.0.0.1)
HTTPServerRequest(protocol='http', host='localhost:5000', method='GET', uri='/api/groups?includeSingleUserGroups=true', version='HTTP/1.0', remote_ip='127.0.0.1')
Traceback (most recent call last):
  File "/home/theodlz/Templates/skyportal/env/lib/python3.9/site-packages/tornado/web.py", line 1702, in _execute
    result = method(*self.path_args, **self.path_kwargs)
  File "/home/theodlz/alright/shift_management_admin_menu/skyportal/baselayer/app/access.py", line 44, in wrapper
    return tornado.web.authenticated(method)(self, *args, **kwargs)
  File "/home/theodlz/Templates/skyportal/env/lib/python3.9/site-packages/tornado/web.py", line 3173, in wrapper
    return method(self, *args, **kwargs)
  File "/home/theodlz/alright/shift_management_admin_menu/skyportal/skyportal/handlers/api/group.py", line 202, in get
    list(self.current_user.groups), key=lambda g: g.name.lower()
  File "/home/theodlz/Templates/skyportal/env/lib/python3.9/site-packages/sqlalchemy/orm/attributes.py", line 481, in __get__
    return self.impl.get(state, dict_)
  File "/home/theodlz/Templates/skyportal/env/lib/python3.9/site-packages/sqlalchemy/orm/attributes.py", line 941, in get
    value = self._fire_loader_callables(state, key, passive)
  File "/home/theodlz/Templates/skyportal/env/lib/python3.9/site-packages/sqlalchemy/orm/attributes.py", line 977, in _fire_loader_callables
    return self.callable_(state, passive)
  File "/home/theodlz/Templates/skyportal/env/lib/python3.9/site-packages/sqlalchemy/orm/strategies.py", line 861, in _load_for_state
    raise orm_exc.DetachedInstanceError(
sqlalchemy.orm.exc.DetachedInstanceError: Parent instance <User at 0x7fc28e2f02e0> is not bound to a Session; lazy load operation of attribute 'groups' cannot proceed (Background on this error at: https://sqlalche.me/e/14/bhk3)
ERROR:tornado.access:500 GET /api/groups?includeSingleUserGroups=true (127.0.0.1) 12.77ms
mcoughlin commented 2 years ago

@Theodlz @guynir42 is working through exactly like this currently.

guynir42 commented 2 years ago

@Theodlz do not pin the latest baselayer. It is not ready to work with skyportal yet.