Flask-AppBuilder
Flask-AppBuilder copied to clipboard
Refactor logging to use lazy interpolation
While not a bug, it seems like the logging behavior of Flask-AppBuilder is not very consistent. Some logging is done with %
formatting, some with .format
function call, some with f-strings, and some with lazy interpolation, the last of which is pedantically the "correct" way to log.
Would it make sense to refactor all logging function calls in Flask-AppBuilder to use lazy interpolation? Here is an example of lazy interpolation already being used: https://github.com/dpgaspar/Flask-AppBuilder/blob/4f9873af0318aa0b39e290b77f6f140315bc3d37/flask_appbuilder/models/sqla/filters.py#L254
class FilterRelationManyToManyEqual(FilterRelation):
name = lazy_gettext("Relation as Many")
arg_name = "rel_m_m"
def apply_item(self, query, field, value_item):
"""
Get object by column_name and value_item, then apply filter if object exists
Query with new filter applied
"""
try:
rel_obj = self.datamodel.get_related_obj(self.column_name, value_item)
except SQLAlchemyError as exc:
logging.warning(
"Filter exception for %s with value %s, will not apply",
field,
value_item,
)
try:
self.datamodel.session.rollback()
except SQLAlchemyError:
# on MSSQL a rollback would fail here
pass
raise ApplyFilterException(exception=exc)
if rel_obj:
return query.filter(field.contains(rel_obj))
else:
log.error(
"Related object for column: %s, value: %s return Null",
self.column_name,
value_item,
)
return query
it would, totally agree
A quick grep revealed the following places where log
is called:
- [x] ./flask_appbuilder/urltools.py
- [x] ./flask_appbuilder/security/registerviews.py
- [x] ./flask_appbuilder/security/sqla/manager.py
- [x] ./flask_appbuilder/security/mongoengine/manager.py
- [x] ./flask_appbuilder/security/manager.py
- [x] ./flask_appbuilder/security/views.py
- [x] ./flask_appbuilder/security/decorators.py
- [x] ./flask_appbuilder/tests/test_fab_cli.py
- [x] ./flask_appbuilder/tests/test_custom_indexview.py
- [x] ./flask_appbuilder/tests/test_mongoengine.py
- [x] ./flask_appbuilder/tests/test_mvc.py
- [x] ./flask_appbuilder/baseviews.py
- [x] ./flask_appbuilder/utils/base.py
- [x] ./flask_appbuilder/models/sqla/interface.py
- [x] ./flask_appbuilder/models/sqla/filters.py
- [x] ./flask_appbuilder/models/mongoengine/interface.py
- [x] ./flask_appbuilder/models/group.py
- [x] ./flask_appbuilder/models/filters.py
- [x] ./flask_appbuilder/forms.py
- [x] ./flask_appbuilder/static/appbuilder/js/bootstrap.min.js
- [x] ./flask_appbuilder/api/init.py
- [x] ./flask_appbuilder/base.py
- [x] ./flask_appbuilder/views.py
- [x] ./bin/hash_db_password.py
- [x] ./bin/migrate_db_0.7.py
- [x] ./bin/migrate_db_1.3.py
- [x] ./examples/basefilter/testdata.py
- [x] ./examples/quicksqlviews/testdata.py
- [x] ./examples/related_fields/testdata.py
- [x] ./examples/user_registration/app/views.py
- [x] ./examples/composite_keys/testdata.py
- [x] ./examples/enums/testdata.py
- [x] ./examples/quickcharts2/app/views.py
- [x] ./examples/quickcharts2/build/bdist.linux-i686/egg/app/views.py
- [x] ./examples/quickcharts2/build/lib/app/views.py
- [x] ./examples/quickmigrate/testdata.py
- [x] ./examples/quickcharts/app/data.py
- [x] ./examples/crud_rest_api/testdata.py
- [x] ./examples/extendsecurity/testdata.py
- [x] ./examples/quickhowto3/.coverage
- [x] ./examples/quickhowto3/migrate_db_0.7.py
- [x] ./examples/extendsecurity2/testdata.py
- [x] ./examples/quickhowto2/app/templates/new_base.html
- [x] ./examples/react-rest-api/testdata.py
- [x] ./examples/quickhowto/testdata.py
Will check off individual file as they are migrated to using lazy interpolation