Incubator-superset: FAB generates empty permissions

[andromedarabbit]

Make sure these boxes are checked before submitting your issue - thank you!

• [/] I have checked the superset logs for python stacktraces and included it here as text if any
• [/] I have reproduced the issue with at least the latest released version of superset
• [] I have checked the issue tracker for the same issue and I haven't found one similar

Superset version

0.20.1

Expected results

Upgrade should be done without any errors

Actual results

superset db upgrade failed with the following messages:

Loaded your LOCAL configuration at [/home/superset/superset_config.py]
2017-11-02 11:29:23,393:ERROR:flask_appbuilder.base:'NoneType' object has no attribute 'name'
Traceback (most recent call last):
  File "/usr/local/lib/python3.5/site-packages/flask_appbuilder/base.py", line 462, in _add_permission
    self.sm.add_permissions_view(baseview.base_permissions, baseview.__class__.__name__)
  File "/usr/local/lib/python3.5/site-packages/flask_appbuilder/security/manager.py", line 804, in add_permissions_view
    if perm_view.permission.name not in base_permissions:
AttributeError: 'NoneType' object has no attribute 'name'
2017-11-02 11:29:23,394:ERROR:flask_appbuilder.base:Add Permission on View Error: 'NoneType' object has no attribute 'name'
2017-11-02 11:29:24,042:ERROR:flask_appbuilder.base:'NoneType' object has no attribute 'name'
Traceback (most recent call last):
  File "/usr/local/lib/python3.5/site-packages/flask_appbuilder/base.py", line 462, in _add_permission
    self.sm.add_permissions_view(baseview.base_permissions, baseview.__class__.__name__)
  File "/usr/local/lib/python3.5/site-packages/flask_appbuilder/security/manager.py", line 804, in add_permissions_view
    if perm_view.permission.name not in base_permissions:
AttributeError: 'NoneType' object has no attribute 'name'
2017-11-02 11:29:24,042:ERROR:flask_appbuilder.base:Add Permission on View Error: 'NoneType' object has no attribute 'name'
INFO  [alembic.runtime.migration] Context impl MySQLImpl.
INFO  [alembic.runtime.migration] Will assume transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade ca69c70ec99b -> ddd6ebdd853b, annotations
INFO  [alembic.runtime.migration] Running upgrade a9c47e2c1547, ddd6ebdd853b -> d39b1e37131d, empty message
INFO  [alembic.runtime.migration] Running upgrade ca69c70ec99b -> 19a814813610, Adding metric warning_text
INFO  [alembic.runtime.migration] Running upgrade 19a814813610, a9c47e2c1547 -> 472d2f73dfd4, empty message
INFO  [alembic.runtime.migration] Running upgrade 472d2f73dfd4, d39b1e37131d -> f959a6652acd, empty message

After superset db upgrade being failed, I found weird records from the table ab_permission_view:

Steps to reproduce

[mistercrunch]

There's an issue where somehow FAB gets in a bad state with perms which explains the stack trace, though it looks like the db upgrade might have succeeded despite the error msg.

[mistercrunch]

Until this gets fixed in FAB, we should make sure that superset init cleans those up. You can manually delete where permission_id is null.

[andromedarabbit]

Thanks for the advice @mistercrunch !

Should I close this or keep this open until FAB is fixed?

[mistercrunch]

I renamed the task, we can keep it open to track this problem

[alanmcruickshank]

@mistercrunch

We periodically get this issue when we upgrade between version too. Happy to do some legwork to add that into superset init. We don't currently run that on upgrade however - do you recommend running it after every version upgrade?

[mistercrunch]

Yes superset init needs to run as some point to create new perms around new views or models and associating them to our roles.

Actually at the moment starting up FAB does some of that work (creating new views and models perms), but that's an issue as when we start many gunicorn workers at once it creates race conditions that ultimately leads to this bad state. That's my guess anyways as I don't have hard evidence.

Based on that intuition I added this feature to FAB to make perm management optional on startup
https://github.com/dpgaspar/Flask-AppBuilder/pull/625

We should switch this option to be the default in Superset and get superset init to trigger that instead.

So short answer is superset init should be executed once per deploy, probably right after superset db upgrade.

[oripwk]

I'm having exactly the same issue but I don't have any NULLs in ab_permission_view. What should I do?

[rickywu]

run init again can solve this error

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.