pierrejeambrun commented on code in PR #74663:
URL: https://github.com/apache/airflow/pull/74663#discussion_r4402986968
##########
airflow-core/src/airflow/ui/vite.config.ts:
##########
@@ -76,8 +76,10 @@ export default defineConfig({
environment: "happy-dom",
exclude: ["**/node_modules/**", "**/dist/**", "tests/e2e/**"],
globals: true,
+ maxWorkers: 4,
Review Comment:
I checked these locally: without the three settings 27 tests time out, and
even with the longer timeouts, dropping `maxWorkers` still leaves 2 failures on
my machine. So tests that render the full router now depend on how loaded the
machine is, and these settings cap parallelism and make every failing `waitFor`
wait 10s for the whole suite. Do you think we could keep the extra time to the
tests that render the router (for example by preloading the route modules
there) rather than changing it globally? Not blocking, but I'm worried about CI
flakiness.
##########
airflow-core/src/airflow/ui/src/router.tsx:
##########
@@ -16,289 +16,356 @@
* specific language governing permissions and limitations
* under the License.
*/
+import type { ComponentType } from "react";
+
import { queryOptions } from "@tanstack/react-query";
import { createBrowserRouter } from "react-router-dom";
import { UseConfigServiceGetConfigsKeyFn } from "openapi/queries";
import { ConfigService } from "openapi/requests/services.gen";
import { BaseLayout } from "src/layouts/BaseLayout";
-import { DagsLayout } from "src/layouts/DagsLayout";
-import { Asset } from "src/pages/Asset";
-import { AssetEvents } from "src/pages/Asset/AssetEvents";
-import { AssetStateStore } from "src/pages/Asset/AssetStateStore";
-import { AssetsList } from "src/pages/AssetsList";
-import { Configs } from "src/pages/Configs";
-import { Connections } from "src/pages/Connections";
-import { Dag } from "src/pages/Dag";
-import { Backfills } from "src/pages/Dag/Backfills";
-import { Calendar } from "src/pages/Dag/Calendar/Calendar";
-import { Code } from "src/pages/Dag/Code";
-import { Details as DagDetails } from "src/pages/Dag/Details";
-import { Overview } from "src/pages/Dag/Overview";
-import { Tasks } from "src/pages/Dag/Tasks";
-import { Versions } from "src/pages/Dag/Versions";
-import { DagBundle } from "src/pages/DagBundle";
-import { DagBundles } from "src/pages/DagBundles";
-import { DagRuns } from "src/pages/DagRuns";
-import { DagsList } from "src/pages/DagsList";
-import { Dashboard } from "src/pages/Dashboard";
-import { Deadlines } from "src/pages/Deadlines";
import { ErrorPage } from "src/pages/Error";
-import { Events } from "src/pages/Events";
-import { ExternalView } from "src/pages/ExternalView";
-import { GroupTaskInstance, GroupTaskInstances } from
"src/pages/GroupTaskInstance";
-import { HITLTaskInstances } from "src/pages/HITLTaskInstances";
-import { Jobs } from "src/pages/Jobs";
-import { LandingPage } from "src/pages/LandingPage";
-import { MappedTaskInstance } from "src/pages/MappedTaskInstance";
-import { Details as MappedTaskInstanceDetails } from
"src/pages/MappedTaskInstance/Details";
-import { Plugins } from "src/pages/Plugins";
-import { Pools } from "src/pages/Pools";
-import { Providers } from "src/pages/Providers";
-import { Run } from "src/pages/Run";
-import { AssetEvents as DagRunAssetEvents } from "src/pages/Run/AssetEvents";
-import { Callbacks as DagRunCallbacks } from "src/pages/Run/Callbacks";
-import { Details as DagRunDetails } from "src/pages/Run/Details";
-import { Security } from "src/pages/Security";
-import { Settings } from "src/pages/Settings";
-import { Task } from "src/pages/Task";
-import { Overview as TaskOverview } from "src/pages/Task/Overview";
-import { TaskInstance, Logs } from "src/pages/TaskInstance";
-import { AssetEvents as TaskInstanceAssetEvents } from
"src/pages/TaskInstance/AssetEvents";
-import { DefaultTab as TaskInstanceDefaultTab } from
"src/pages/TaskInstance/DefaultTab";
-import { Details as TaskInstanceDetails } from
"src/pages/TaskInstance/Details";
-import { HITLResponse } from "src/pages/TaskInstance/HITLResponse";
-import { RenderedTemplates } from "src/pages/TaskInstance/RenderedTemplates";
-import { TaskInstances } from "src/pages/TaskInstances";
-import { TaskStateStore } from "src/pages/TaskStateStore";
-import { Variables } from "src/pages/Variables";
-import { XCom } from "src/pages/XCom";
import { TabEntity, TabName, TaskInstanceTab } from "src/constants/tab";
import { client } from "./queryClient";
+const ROUTE_MODULE_RELOAD_KEY = "airflow-route-module-reload-attempted";
+
+export const loadRouteModule = async <Module,>(factory: () => Promise<Module>)
=> {
+ try {
+ return await factory();
+ } catch (error) {
Review Comment:
I tried this by making the route chunks 404 and found two things. The reload
happens before React Router commits the new URL, so it reloads the page the
user was leaving and the click is lost. And since the flag is never cleared, a
second redeploy later in the same tab gets no automatic reload, only the error
page. Could we clear the flag once a module loads?
```suggestion
export const loadRouteModule = async <Module,>(factory: () =>
Promise<Module>) => {
try {
const module = await factory();
globalThis.sessionStorage.removeItem(ROUTE_MODULE_RELOAD_KEY);
return module;
} catch (error) {
```
For the first part, maybe the one-time reload could move into `ErrorPage`?
It renders after the URL has changed, so the reload would land on the page the
user asked for.
--
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]
