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.

Sign up for GitHub

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

Jump to bottom

Bump FAB to 2.0.0 #7323

Merged
merged 11 commits into from
Apr 30, 2019
Merged

Bump FAB to 2.0.0 #7323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bb96a8d
Bump FAB to 2.0.0
dpgaspar Apr 18, 2019
c9c2dd0
[tests] whitelist SecurityApi login and refresh endpoints
dpgaspar Apr 18, 2019
21fa309
[style] Fix, C812 missing trailing commas
dpgaspar Apr 18, 2019
e2eec2f
[security] Remove SUPERSET_UPDATE_PERMS flag
dpgaspar Apr 19, 2019
19aa5e2
[docs] New, FAB_UPDATE_PERMS and flask fab cli
dpgaspar Apr 19, 2019
b39a0d6
Merge branch 'master' into fab_2.0.0
dpgaspar Apr 19, 2019
87523c4
[docs] Fix, db upgrade needs to come first, create-admin needs a db
dpgaspar Apr 19, 2019
6b34e12
[cli] New, superset init bootstraps all permissions for FAB and Superset
dpgaspar Apr 23, 2019
e0c0d3e
[style] Fix, flakes
dpgaspar Apr 23, 2019
5885884
Merge branch 'master' into fab_2.0.0
dpgaspar Apr 24, 2019
4b37cac
Merge branch 'master' into fab_2.0.0
mistercrunch Apr 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions UPDATING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ under the License.
This file documents any backwards-incompatible changes in Superset and
assists people when migrating to a new version.

## Superset 0.34.0
## Next Version

* [5451](https://github.com/apache/incubator-superset/pull/5451): a change
which adds missing non-nullable fields to the `datasources` table. Depending on
Expand All @@ -31,7 +31,11 @@ the integrity of the data, manual intervention may be required.
which adds missing non-nullable fields and uniqueness constraints to the
`columns`and `table_columns` tables. Depending on the integrity of the data,
manual intervention may be required.

* `fabmanager` command line is deprecated since Flask-AppBuilder 2.0.0, use
the new `flask fab <command>` integrated with *Flask cli*.
* `SUPERSET_UPDATE_PERMS` environment variable was replaced by
`FAB_UPDATE_PERMS` config boolean key. To disable automatic
creation of permissions set `FAB_UPDATE_PERMS = False` on config.
* [5453](https://github.com/apache/incubator-superset/pull/5453): a change
which adds missing non-nullable fields and uniqueness constraints to the metrics
and sql_metrics tables. Depending on the integrity of the data, manual
Expand Down
4 changes: 2 additions & 2 deletions docs/faq.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,8 @@ by setting the ``stagger_refresh`` to ``false`` and modify the stagger period by
Here, the entire dashboard will refresh at once if periodic refresh is on. The stagger time of
2.5 seconds is ignored.

Why does fabmanager or superset freezed/hung/not responding when started (my home directory is NFS mounted)?
------------------------------------------------------------------------------------------------------------
Why does 'flask fab' or superset freezed/hung/not responding when started (my home directory is NFS mounted)?
-------------------------------------------------------------------------------------------------------------
By default, superset creates and uses an sqlite database at ``~/.superset/superset.db``. Sqlite is known to `don't work well if used on NFS`__ due to broken file locking implementation on NFS.

__ https://www.sqlite.org/lockingv3.html
Expand Down
17 changes: 6 additions & 11 deletions docs/installation.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,20 +170,21 @@ Follow these few simple steps to install Superset.::
# Install superset
pip install superset

# Create an admin user (you will be prompted to set a username, first and last name before setting a password)
fabmanager create-admin --app superset

# Initialize the database
superset db upgrade

# Create an admin user (you will be prompted to set a username, first and last name before setting a password)
$ export FLASK_APP=superset
flask fab create-admin

# Load some data to play with
superset load_examples

# Create default roles and permissions
superset init

# To start a development web server on port 8088, use -p to bind to another port
superset runserver -d
flask run -p 8080 --with-threads --reload --debugger


After installation, you should be able to point your browser to the right
Expand Down Expand Up @@ -236,17 +237,11 @@ workers this creates a lot of contention and race conditions when defining
permissions and views.

To alleviate this issue, the automatic updating of permissions can be disabled
by setting the environment variable
`SUPERSET_UPDATE_PERMS` environment variable to `0`.
The value `1` enables it, `0` disables it. Note if undefined the functionality
is enabled to maintain backwards compatibility.
by setting `FAB_UPDATE_PERMS = False` (defaults to True).

In a production environment initialization could take on the following form:

export SUPERSET_UPDATE_PERMS=1
superset init

export SUPERSET_UPDATE_PERMS=0
gunicorn -w 10 ... superset:app

Configuration behind a load balancer
Expand Down
17 changes: 13 additions & 4 deletions requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
#
alembic==1.0.0 # via flask-migrate
amqp==2.3.2 # via kombu
apispec[yaml]==1.2.0 # via flask-appbuilder
asn1crypto==0.24.0 # via cryptography
attrs==19.1.0 # via jsonschema
babel==2.6.0 # via flask-babel
billiard==3.5.0.4 # via celery
bleach==3.0.2
Expand All @@ -21,10 +23,11 @@ croniter==0.3.29
cryptography==2.4.2
decorator==4.3.0 # via retry
defusedxml==0.5.0 # via python3-openid
flask-appbuilder==1.12.5
flask-appbuilder==2.0.0
flask-babel==0.11.1 # via flask-appbuilder
flask-caching==1.4.0
flask-compress==1.4.0
flask-jwt-extended==3.18.1 # via flask-appbuilder
flask-login==0.4.1 # via flask-appbuilder
flask-migrate==2.1.1
flask-openid==1.2.5 # via flask-appbuilder
Expand All @@ -38,19 +41,25 @@ idna==2.6
isodate==0.6.0
itsdangerous==0.24 # via flask
jinja2==2.10 # via flask, flask-babel
jsonschema==3.0.1 # via flask-appbuilder
kombu==4.2.1 # via celery
mako==1.0.7 # via alembic
markdown==3.0
markupsafe==1.0 # via jinja2, mako
marshmallow-enum==1.4.1 # via flask-appbuilder
marshmallow-sqlalchemy==0.16.2 # via flask-appbuilder
marshmallow==2.19.2 # via flask-appbuilder, marshmallow-enum, marshmallow-sqlalchemy
numpy==1.15.2 # via pandas
pandas==0.23.4
parsedatetime==2.0.0
pathlib2==2.3.0
polyline==1.3.2
prison==0.1.0 # via flask-appbuilder
py==1.7.0 # via retry
pycparser==2.19 # via cffi
pydruid==0.5.2
pyjwt==1.7.1 # via flask-appbuilder
pyjwt==1.7.1 # via flask-appbuilder, flask-jwt-extended
pyrsistent==0.14.11 # via jsonschema
python-dateutil==2.6.1
python-editor==1.0.3 # via alembic
python-geohash==0.8.5
Expand All @@ -61,14 +70,14 @@ requests==2.20.0
retry==0.9.2
selenium==3.141.0
simplejson==3.15.0
six==1.11.0 # via bleach, cryptography, isodate, pathlib2, polyline, pydruid, python-dateutil, sqlalchemy-utils, wtforms-json
six==1.11.0 # via bleach, cryptography, flask-jwt-extended, isodate, jsonschema, pathlib2, polyline, prison, pydruid, pyrsistent, python-dateutil, sqlalchemy-utils, wtforms-json
sqlalchemy-utils==0.32.21
sqlalchemy==1.3.1
sqlparse==0.2.4
unicodecsv==0.14.1
urllib3==1.22 # via requests, selenium
vine==1.1.4 # via amqp
webencodings==0.5.1 # via bleach
werkzeug==0.14.1 # via flask
werkzeug==0.14.1 # via flask, flask-jwt-extended
wtforms-json==0.3.3
wtforms==2.2.1 # via flask-wtf, wtforms-json
2 changes: 1 addition & 1 deletion setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def get_git_sha():
'croniter>=0.3.28',
'cryptography>=2.4.2',
'flask>=1.0.0, <2.0.0',
'flask-appbuilder>=1.12.5, <2.0.0',
'flask-appbuilder>=2.0.0, <2.3.0',
'flask-caching',
'flask-compress',
'flask-migrate',
Expand Down
14 changes: 6 additions & 8 deletions superset/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@
from superset import config
from superset.connectors.connector_registry import ConnectorRegistry
from superset.security import SupersetSecurityManager
from superset.utils.core import (
get_update_perms_flag, pessimistic_connection_handling, setup_cache)
from superset.utils.core import pessimistic_connection_handling, setup_cache

wtforms_json.init()

Expand Down Expand Up @@ -202,7 +201,6 @@ def index(self):
base_template='superset/base.html',
indexview=MyIndexView,
security_manager_class=custom_sm,
update_perms=get_update_perms_flag(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should make this False now, and assume people will have to run superset init once per deploy

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think now we need to call appbuilder.add_permissions(update_perms=True) from superset.cli.init

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FAB defaults to True on update_perms, so that we get the same behavior.
My reasoning is that the present change will create all permissions on start just like before.
If we want to disable the permission creation (avoid contention on the db on startup),
FAB_UPDATE_PERMS=False and run superset init once per deploy.

I would say that appbuilder.add_permissions(update_perms=True) can be achieved by calling the new flask fab create-permissions. I noticed that superset db upgrade creates and upgrades the db, leaving it already with bootstrap permissions and superset init created Superset's extra roles even with FAB_UPDATE_PERMS=False (so all is good).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let's add another note in UPDATE.md about FAB_UPDATE_PERMS = False being the new equivalent to the envvar SUPERSET_UPDATE_PERMS=0.

I think it'd be good to call appbuilder.add_permissions(update_perms=True) within superset init, so that we people don't have to also flask fab create-permissions at every deploy. I don't think using create-permissions is documented anywhere on our side. I think it's easier to make superset init do more instead of documenting using flask fab

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, new commit.

)

security_manager = appbuilder.sm
Expand All @@ -226,11 +224,6 @@ def is_feature_enabled(feature):
return get_feature_flags().get(feature)


# Registering sources
module_datasource_map = app.config.get('DEFAULT_MODULE_DS_MAP')
module_datasource_map.update(app.config.get('ADDITIONAL_MODULE_DS_MAP'))
ConnectorRegistry.register_sources(module_datasource_map)

# Flask-Compress
if conf.get('ENABLE_FLASK_COMPRESS'):
Compress(app)
Expand All @@ -242,3 +235,8 @@ def is_feature_enabled(feature):
flask_app_mutator(app)

from superset import views # noqa

# Registering sources
module_datasource_map = app.config.get('DEFAULT_MODULE_DS_MAP')
module_datasource_map.update(app.config.get('ADDITIONAL_MODULE_DS_MAP'))
ConnectorRegistry.register_sources(module_datasource_map)
3 changes: 2 additions & 1 deletion superset/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import yaml

from superset import (
app, data, db, security_manager,
app, appbuilder, data, db, security_manager,
)
from superset.utils import (
core as utils, dashboard_import_export, dict_import_export)
Expand All @@ -50,6 +50,7 @@ def make_shell_context():
def init():
"""Inits the Superset application"""
utils.get_or_create_main_db()
appbuilder.add_permissions(update_perms=True)
security_manager.sync_role_definitions()


Expand Down
5 changes: 0 additions & 5 deletions superset/utils/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -852,11 +852,6 @@ def merge_request_params(form_data: dict, params: dict):
form_data['url_params'] = url_params


def get_update_perms_flag() -> bool:
val = os.environ.get('SUPERSET_UPDATE_PERMS')
return val.lower() not in ('0', 'false', 'no') if val else True


def user_label(user: User) -> Optional[str]:
"""Given a user ORM FAB object, returns a label"""
if user:
Expand Down
2 changes: 2 additions & 0 deletions tests/security_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,8 @@ def test_views_are_secured(self):
['Superset', 'log'],
['Superset', 'theme'],
['Superset', 'welcome'],
['SecurityApi', 'login'],
['SecurityApi', 'refresh'],
]
unsecured_views = []
for view_class in appbuilder.baseviews:
Expand Down