dpgaspar commented on code in PR #34679:
URL: https://github.com/apache/superset/pull/34679#discussion_r2281427594
##########
superset/initialization/__init__.py:
##########
@@ -503,11 +504,40 @@ def init_views(self) -> None:
icon="fa-lock",
)
+ def _is_database_up_to_date(self) -> bool:
Review Comment:
nit: we could move this to `utils` if you think it could be useful for other
code paths in the future
##########
superset/initialization/__init__.py:
##########
@@ -517,39 +547,24 @@ def _init_database_dependent_features(self) -> None:
):
logger.warning(
"Database URI appears to be a fallback value. "
- "Skipping database-dependent initialization. "
- "This may indicate the workspace context is not ready yet."
+ "Skipping database-dependent initialization."
)
return
- try:
- inspector = inspect(db.engine)
-
- # Check if core tables exist (use 'dashboards' as proxy for
Superset tables)
- if not inspector.has_table("dashboards"):
- logger.debug(
- "Superset tables not yet created. Skipping
database-dependent "
- "initialization. These features will be initialized after "
- "migration."
- )
- return
- except OperationalError as e:
- logger.debug(
- "Error inspecting database tables. Skipping database-dependent
"
- "initialization: %s",
- e,
- )
+ # Check if database is up-to-date with migrations
+ if not self._is_database_up_to_date():
+ logger.info("Pending database migrations: run 'superset db
upgrade'")
return
+ # Initialize all database-dependent features
# Register SQLA event listeners for tagging system
if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
register_sqla_event_listeners()
# Seed system themes from configuration
from superset.commands.theme.seed import SeedSystemThemesCommand
- if inspector.has_table("themes"):
- SeedSystemThemesCommand().run()
+ SeedSystemThemesCommand().run()
Review Comment:
We do `AppBuilder(update_perms=False)` to avoid needing the db on startup
and use the cli to update permissions. We could also use `FAB_UPDATE_PERMS` to
control this behaviour also.
Following the same pattern we could create a new config flag to avoid
seeding the themes at app startup, and move this to `superset init` for example.
I'm also thinking about `register_sqla_event_listeners` that won't register
any listeners if we don't have a request context, and having a request context
is not expected on app initialization, so these listeners won't be registered
if the db uri contains `["nouser", "nopassword", "nohost", "nodb"]` and
`TAGGING_SYSTEM` is `True`
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
