dpgaspar commented on code in PR #34345: URL: https://github.com/apache/superset/pull/34345#discussion_r2244914521
########## superset/advanced_data_type/api.py: ########## @@ -29,8 +29,8 @@ from superset.extensions import event_logger from superset.views.base_api import BaseSupersetApi -config = app.config -ADVANCED_DATA_TYPES = config["ADVANCED_DATA_TYPES"] +conf = app.config Review Comment: I'm not a fan of these module level references, what do you think about removing them and replacing `ADVANCED_DATA_TYPES` with `current_app.config["ADVANCED_DATA_TYPES"]` where needed? ########## superset/commands/dashboard/filter_state/get.py: ########## @@ -16,20 +16,23 @@ # under the License. from typing import Optional -from flask import current_app as app +from flask import current_app from superset.commands.dashboard.filter_state.utils import check_access from superset.commands.temporary_cache.get import GetTemporaryCacheCommand from superset.commands.temporary_cache.parameters import CommandParameters from superset.extensions import cache_manager from superset.temporary_cache.utils import cache_key +conf = current_app.config Review Comment: can we avoid these module level references, they need an app context, so any imports to this module will need an app context also -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
