mistercrunch commented on code in PR #34679:
URL: https://github.com/apache/superset/pull/34679#discussion_r2289422568
##########
superset/app.py:
##########
@@ -96,6 +100,90 @@ def send_static_file(self, filename: str) -> Response:
return Response("", status=204) # No Content
return super().send_static_file(filename)
+ def _is_database_up_to_date(self) -> bool:
+ """
+ Check if database migrations are up to date.
+ Returns False if there are pending migrations or unable to determine.
+ """
+ try:
+ # Import here to avoid circular import issues
+ from superset.extensions import db
+
+ # Get current revision from database
+ with db.engine.connect() as connection:
+ context = MigrationContext.configure(connection)
+ current_rev = context.get_current_revision()
+
+ # Get head revision from migration files
+ alembic_cfg = Config()
+ alembic_cfg.set_main_option("script_location",
"superset:migrations")
+ script = ScriptDirectory.from_config(alembic_cfg)
+ head_rev = script.get_current_head()
+
+ # Database is up-to-date if current revision matches head
+ is_current = current_rev == head_rev
+ if not is_current:
+ logger.debug(
+ "Pending migrations. Current: %s, Head: %s",
+ current_rev,
+ head_rev,
+ )
+ return is_current
+ except Exception as e:
+ logger.debug("Could not check migration status: %s", e)
+ return False
+
+ def sync_config_to_db(self) -> None:
+ """
+ Synchronize configuration to database.
+ This method handles database-dependent features that need to be synced
+ after the app is initialized and database connection is available.
+
+ This is separated from app initialization to support multi-tenant
+ environments where database connection might not be available during
+ app startup.
+ """
+ try:
+ # Import here to avoid circular import issues
+ from superset.extensions import feature_flag_manager
+
+ # Check if database URI is a fallback value before trying to
connect
+ db_uri = self.config.get("SQLALCHEMY_DATABASE_URI", "")
+ if not db_uri or any(
+ fallback in db_uri.lower()
+ for fallback in ["nouser", "nopassword", "nohost", "nodb"]
+ ):
+ logger.warning(
+ "Database URI appears to be a fallback value. "
+ "Skipping database-dependent sync."
+ )
+ return
+
+ # Check if database is up-to-date with migrations
+ if not self._is_database_up_to_date():
Review Comment:
I think it's necessary, the issue that prompted all this was that if you
were on an old migration (pre Theme model creation), everything would fail,
including `superset db upgrade` ...
In general, I think that `sync_config_to_db` requires a check to make sure
that the db is up-to-date. If anything, maybe we should even fail harder and
only allow you to run `superset db upgrade` or things that really don't depend
on the database.
I think this check here makes things better, there's now a clear
`sync_to_db()` step that requires an up-to-date database, pretty important
assumption before syncing anything...
--
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
