From 30cc62a8e08251d78fb82115710b58433fa8e7a9 Mon Sep 17 00:00:00 2001 From: Marco polo Date: Sun, 9 Jun 2024 16:13:09 -1000 Subject: [PATCH] [1/n] Split RootWorkspaceQuery by code location (#22296) ## Summary & Motivation This PR speeds up workspace fetching by splitting the RootWorkspaceQuery into multiple parallel fetches by code location. We now rely on the CodeLocationStatusQuery to determine these locations. To minimize delays, we cache the code location status query in IndexedDB, enabling immediate fetches after the initial load. It also enables us to fetch only changed code locations rather than needing to fetch all locations when a single one changes. The WorkspaceProvider is now responsible for refetching code locations when they change by comparing status updates to detect changes. This responsibility was previously with useCodeLocationsStatus, but to avoid a cyclical dependency, it has been moved to the WorkspaceProvider. useCodeLocationsStatus will continue handling toast updates and returning status information for downstream consumers, but now it uses data from the workspace context instead of making direct queries. (In a future PR we could collapse some of this functionality but I wanted to minimize the number of code changes in this PR). The skip parameter has been removed from useCodeLocationsStatus since it was never set to true in either cloud or OSS environments, both of which poll for code location updates. Tests now require mocking more queries to create the desired workspace context. To simplify this, a buildWorkspaceMocks function has been added. It accepts location entries as arguments and creates the necessary code location statuses query mock and workspace query mocks by location. Had to update testing-library/react to get tests to pass because the version we were on had some goofy react act stuff that was fixed. Next PRs in stack will be: - [Cloud PR updating workspace mocks / imports](https://github.com/dagster-io/internal/pull/10126) - Fetching schedule/sensor state independently in LeftNavItem and useDaemonStatus (currently they depend on the workspace context but this doesnt refresh anymore) - Making OverviewSensors/OverviewSchedules/OverviewJobs etc. use the root workspace instead of fetching everything independently from scratch every time. ## How I Tested These Changes - Ran local host cloud and updated code locations. Tested cached loads with locations different and with locations the same making sure we didn't make extra queries if not necessary - Ran dagster-dev and made sure manual code location reloading still worked - Existing indexeddb test still pass covering the changes in that file - Relying on existing tests using WorkspaceProvider to make sure product functionality is still intact. A cloud PR will follow updating workspace mocks there --- .../packages/ui-components/package.json | 2 +- .../dagster-ui/packages/ui-core/package.json | 2 +- .../AssetViewDefinition.fixtures.ts | 42 +- .../src/assets/__tests__/AssetTables.test.tsx | 7 +- .../src/assets/__tests__/AssetView.test.tsx | 2 +- ...LaunchAssetChoosePartitionsDialog.test.tsx | 34 +- .../LaunchAssetExecutionButton.test.tsx | 7 +- .../src/instance/DeploymentStatusProvider.tsx | 2 +- .../backfill/__tests__/BackfillTable.test.tsx | 11 +- .../src/instance/flattenCodeLocationRows.tsx | 2 +- .../LeftNavRepositorySection.fixtures.tsx | 191 +++----- .../__fixtures__/useDaemonStatus.fixtures.tsx | 92 ++-- .../LeftNavRepositorySection.test.tsx | 49 +- .../nav/__tests__/useDaemonStatus.test.tsx | 48 +- .../nav/types/useCodeLocationsStatus.types.ts | 20 - .../src/nav/useCodeLocationsStatus.tsx | 129 ++--- .../ui-core/src/overview/OverviewSensors.tsx | 19 +- .../runs/HourlyDataCache/HourlyDataCache.tsx | 2 +- .../__fixtures__/RunActionsMenu.fixtures.tsx | 59 +-- .../__fixtures__/RunsFilterInput.fixtures.tsx | 18 +- .../runs/__tests__/RunActionButtons.test.tsx | 2 +- .../runs/__tests__/RunActionsMenu.test.tsx | 4 +- .../runs/__tests__/RunFilterInput.test.tsx | 43 +- .../useIndexedDBCachedQuery.test.tsx | 1 + .../src/search/useIndexedDBCachedQuery.tsx | 247 ++++++---- .../src/workspace/CodeLocationRowSet.tsx | 84 +--- .../workspace/VirtualizedCodeLocationRow.tsx | 2 +- .../src/workspace/WorkspaceContext.tsx | 461 ++++++++++-------- .../src/workspace/WorkspaceQueries.tsx | 140 ++++++ .../__fixtures__/Workspace.fixtures.ts | 62 +++ .../__tests__/WorkspaceContext.test.tsx | 436 +++++++++++++++++ .../getFeatureFlagForCodeLocation.test.tsx | 2 +- ...ext.types.ts => WorkspaceQueries.types.ts} | 224 ++++----- js_modules/dagster-ui/yarn.lock | 24 +- 34 files changed, 1481 insertions(+), 989 deletions(-) delete mode 100644 js_modules/dagster-ui/packages/ui-core/src/nav/types/useCodeLocationsStatus.types.ts create mode 100644 js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceQueries.tsx create mode 100644 js_modules/dagster-ui/packages/ui-core/src/workspace/__fixtures__/Workspace.fixtures.ts create mode 100644 js_modules/dagster-ui/packages/ui-core/src/workspace/__tests__/WorkspaceContext.test.tsx rename js_modules/dagster-ui/packages/ui-core/src/workspace/types/{WorkspaceContext.types.ts => WorkspaceQueries.types.ts} (66%) diff --git a/js_modules/dagster-ui/packages/ui-components/package.json b/js_modules/dagster-ui/packages/ui-components/package.json index 6a8e2e34e44ba..b94b524083825 100644 --- a/js_modules/dagster-ui/packages/ui-components/package.json +++ b/js_modules/dagster-ui/packages/ui-components/package.json @@ -66,7 +66,7 @@ "@storybook/react-webpack5": "^7.2.0", "@testing-library/dom": "^10.0.0", "@testing-library/jest-dom": "^6.4.2", - "@testing-library/react": "^15.0.3", + "@testing-library/react": "^16.0.0", "@testing-library/user-event": "^14.5.2", "@types/babel__core": "^7", "@types/babel__preset-env": "^7", diff --git a/js_modules/dagster-ui/packages/ui-core/package.json b/js_modules/dagster-ui/packages/ui-core/package.json index d4a10b3dcb97b..08e3d9b9dc9c4 100644 --- a/js_modules/dagster-ui/packages/ui-core/package.json +++ b/js_modules/dagster-ui/packages/ui-core/package.json @@ -107,7 +107,7 @@ "@storybook/react-webpack5": "^7.6.7", "@testing-library/dom": "^10.0.0", "@testing-library/jest-dom": "^6.4.2", - "@testing-library/react": "^15.0.3", + "@testing-library/react": "^16.0.0", "@testing-library/user-event": "^14.5.2", "@types/codemirror": "^5.60.5", "@types/color": "^3.0.2", diff --git a/js_modules/dagster-ui/packages/ui-core/src/assets/__fixtures__/AssetViewDefinition.fixtures.ts b/js_modules/dagster-ui/packages/ui-core/src/assets/__fixtures__/AssetViewDefinition.fixtures.ts index 49e55730e8142..e1e76ac2e58d9 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/assets/__fixtures__/AssetViewDefinition.fixtures.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/assets/__fixtures__/AssetViewDefinition.fixtures.ts @@ -6,14 +6,9 @@ import { buildRepository, buildRepositoryLocation, buildResourceRequirement, - buildWorkspace, buildWorkspaceLocationEntry, } from '../../graphql/types'; -import {ROOT_WORKSPACE_QUERY} from '../../workspace/WorkspaceContext'; -import { - RootWorkspaceQuery, - RootWorkspaceQueryVariables, -} from '../../workspace/types/WorkspaceContext.types'; +import {buildWorkspaceMocks} from '../../workspace/__fixtures__/Workspace.fixtures'; import {ASSET_VIEW_DEFINITION_QUERY} from '../AssetView'; import {buildQueryMock} from '../AutoMaterializePolicyPage/__fixtures__/AutoMaterializePolicyPage.fixtures'; import { @@ -153,30 +148,19 @@ export const AssetViewDefinitionSDA = buildQueryMock< }, }); -export const RootWorkspaceWithOneLocation = buildQueryMock< - RootWorkspaceQuery, - RootWorkspaceQueryVariables ->({ - query: ROOT_WORKSPACE_QUERY, - variables: {}, - data: { - workspaceOrError: buildWorkspace({ - locationEntries: [ - buildWorkspaceLocationEntry({ - locationOrLoadError: buildRepositoryLocation({ - repositories: [ - buildRepository({ - id: '4d0b1967471d9a4682ccc97d12c1c508d0d9c2e1', - name: 'repo', - location: buildRepositoryLocation({ - id: 'test.py', - name: 'test.py', - }), - }), - ], +export const RootWorkspaceWithOneLocation = buildWorkspaceMocks([ + buildWorkspaceLocationEntry({ + locationOrLoadError: buildRepositoryLocation({ + repositories: [ + buildRepository({ + id: '4d0b1967471d9a4682ccc97d12c1c508d0d9c2e1', + name: 'repo', + location: buildRepositoryLocation({ + id: 'test.py', + name: 'test.py', }), }), ], }), - }, -}); + }), +]); diff --git a/js_modules/dagster-ui/packages/ui-core/src/assets/__tests__/AssetTables.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/assets/__tests__/AssetTables.test.tsx index 96f06dd2888f4..99cf34cb1255d 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/assets/__tests__/AssetTables.test.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/assets/__tests__/AssetTables.test.tsx @@ -4,9 +4,8 @@ import userEvent from '@testing-library/user-event'; import {MemoryRouter} from 'react-router'; import {RecoilRoot} from 'recoil'; -import {buildWorkspace} from '../../graphql/types'; -import {buildWorkspaceContextMockedResponse} from '../../runs/__fixtures__/RunsFilterInput.fixtures'; import {WorkspaceProvider} from '../../workspace/WorkspaceContext'; +import {buildWorkspaceMocks} from '../../workspace/__fixtures__/Workspace.fixtures'; import {AssetsCatalogTable} from '../AssetsCatalogTable'; import { AssetCatalogGroupTableMock, @@ -17,7 +16,7 @@ import { SingleAssetQueryTrafficDashboard, } from '../__fixtures__/AssetTables.fixtures'; -const workspaceMock = buildWorkspaceContextMockedResponse(buildWorkspace({})); +const workspaceMocks = buildWorkspaceMocks([]); const MOCKS = [ AssetCatalogTableMock, @@ -26,7 +25,7 @@ const MOCKS = [ SingleAssetQueryMaterializedWithLatestRun, SingleAssetQueryMaterializedStaleAndLate, SingleAssetQueryLastRunFailed, - workspaceMock, + ...workspaceMocks, ]; // This file must be mocked because Jest can't handle `import.meta.url`. diff --git a/js_modules/dagster-ui/packages/ui-core/src/assets/__tests__/AssetView.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/assets/__tests__/AssetView.test.tsx index 4617116d1c57f..b4a2eaa25ff1c 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/assets/__tests__/AssetView.test.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/assets/__tests__/AssetView.test.tsx @@ -59,7 +59,7 @@ describe('AssetView', () => { { it('Adding a dynamic partition when multiple assets selected', async () => { @@ -92,21 +92,23 @@ describe('launchAssetChoosePartitionsDialog', () => { assetASecondQueryMock, assetBSecondQueryMock, addPartitionMock, - workspaceMock, + ...workspaceMocks, ]} > - {}} - repoAddress={buildRepoAddress('test', 'test')} - target={{ - jobName: '__ASSET_JOB_0', - partitionSetName: '__ASSET_JOB_0_partition_set', - type: 'job', - }} - assets={[assetA, assetB]} - upstreamAssetKeys={[]} - /> + + {}} + repoAddress={buildRepoAddress('test', 'test')} + target={{ + jobName: '__ASSET_JOB_0', + partitionSetName: '__ASSET_JOB_0_partition_set', + type: 'job', + }} + assets={[assetA, assetB]} + upstreamAssetKeys={[]} + /> + , ); diff --git a/js_modules/dagster-ui/packages/ui-core/src/assets/__tests__/LaunchAssetExecutionButton.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/assets/__tests__/LaunchAssetExecutionButton.test.tsx index 38e73ecc8d12a..7a2a14d345fc0 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/assets/__tests__/LaunchAssetExecutionButton.test.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/assets/__tests__/LaunchAssetExecutionButton.test.tsx @@ -6,12 +6,11 @@ import userEvent from '@testing-library/user-event'; import {CustomAlertProvider} from '../../app/CustomAlertProvider'; import {CustomConfirmationProvider} from '../../app/CustomConfirmationProvider'; import {displayNameForAssetKey} from '../../asset-graph/Utils'; -import {buildWorkspace} from '../../graphql/types'; import {LaunchPartitionBackfillMutation} from '../../instance/backfill/types/BackfillUtils.types'; -import {buildWorkspaceContextMockedResponse} from '../../runs/__fixtures__/RunsFilterInput.fixtures'; import {LaunchPipelineExecutionMutation} from '../../runs/types/RunUtils.types'; import {TestProvider} from '../../testing/TestProvider'; import * as WorkspaceContext from '../../workspace/WorkspaceContext'; +import {buildWorkspaceMocks} from '../../workspace/__fixtures__/Workspace.fixtures'; import {ADDITIONAL_REQUIRED_KEYS_WARNING} from '../AssetDefinedInMultipleReposNotice'; import { AssetsInScope, @@ -46,7 +45,7 @@ import { } from '../__fixtures__/LaunchAssetExecutionButton.fixtures'; import {asAssetKeyInput} from '../asInput'; -const workspaceMock = buildWorkspaceContextMockedResponse(buildWorkspace({})); +const workspaceMocks = buildWorkspaceMocks([]); // This file must be mocked because Jest can't handle `import.meta.url`. jest.mock('../../graph/asyncGraphLayout', () => ({})); @@ -653,7 +652,7 @@ function renderButton({ }), buildLaunchAssetLoaderMock([MULTI_ASSET_OUT_1.assetKey, MULTI_ASSET_OUT_2.assetKey]), buildLaunchAssetLoaderMock(assetKeys), - workspaceMock, + ...workspaceMocks, ...(launchMock ? [launchMock] : []), ]; diff --git a/js_modules/dagster-ui/packages/ui-core/src/instance/DeploymentStatusProvider.tsx b/js_modules/dagster-ui/packages/ui-core/src/instance/DeploymentStatusProvider.tsx index 2ad2305371ab9..1f65f1913d09d 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/instance/DeploymentStatusProvider.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/instance/DeploymentStatusProvider.tsx @@ -24,7 +24,7 @@ interface Props { export const DeploymentStatusProvider = (props: Props) => { const {children, include} = props; - const codeLocations = useCodeLocationsStatus(!include.has('code-locations')); + const codeLocations = useCodeLocationsStatus(); const daemons = useDaemonStatus(!include.has('daemons')); const value = React.useMemo(() => ({codeLocations, daemons}), [daemons, codeLocations]); diff --git a/js_modules/dagster-ui/packages/ui-core/src/instance/backfill/__tests__/BackfillTable.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/instance/backfill/__tests__/BackfillTable.test.tsx index fce0aa4dc0331..d0c0e314a095b 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/instance/backfill/__tests__/BackfillTable.test.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/instance/backfill/__tests__/BackfillTable.test.tsx @@ -1,6 +1,5 @@ import {MockedProvider} from '@apollo/client/testing'; -import {render, screen} from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; +import {act, render, screen, waitFor} from '@testing-library/react'; import {MemoryRouter} from 'react-router-dom'; import * as Alerting from '../../../app/CustomAlertProvider'; @@ -28,7 +27,11 @@ describe('BackfillTable', () => { expect(screen.getByRole('table')).toBeVisible(); const statusLabel = await screen.findByText('Failed'); - await userEvent.click(statusLabel); - expect(Alerting.showCustomAlert).toHaveBeenCalled(); + act(() => { + statusLabel.click(); + }); + await waitFor(() => { + expect(Alerting.showCustomAlert).toHaveBeenCalled(); + }); }); }); diff --git a/js_modules/dagster-ui/packages/ui-core/src/instance/flattenCodeLocationRows.tsx b/js_modules/dagster-ui/packages/ui-core/src/instance/flattenCodeLocationRows.tsx index e316e1a5701d0..b9d7513e2dd71 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/instance/flattenCodeLocationRows.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/instance/flattenCodeLocationRows.tsx @@ -1,5 +1,5 @@ import {CodeLocationRowType} from '../workspace/VirtualizedCodeLocationRow'; -import {WorkspaceLocationNodeFragment} from '../workspace/types/WorkspaceContext.types'; +import {WorkspaceLocationNodeFragment} from '../workspace/types/WorkspaceQueries.types'; const flatten = (locationEntries: WorkspaceLocationNodeFragment[]) => { // Consider each loaded repository to be a "code location". diff --git a/js_modules/dagster-ui/packages/ui-core/src/nav/__fixtures__/LeftNavRepositorySection.fixtures.tsx b/js_modules/dagster-ui/packages/ui-core/src/nav/__fixtures__/LeftNavRepositorySection.fixtures.tsx index 506fb2014bb1b..34381877df7e0 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/nav/__fixtures__/LeftNavRepositorySection.fixtures.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/nav/__fixtures__/LeftNavRepositorySection.fixtures.tsx @@ -1,16 +1,13 @@ -import {MockedResponse} from '@apollo/client/testing'; - import { + WorkspaceLocationEntry, buildAssetGroup, buildPipeline, buildRepository, buildRepositoryLocation, - buildWorkspace, buildWorkspaceLocationEntry, } from '../../graphql/types'; -import {ROOT_WORKSPACE_QUERY} from '../../workspace/WorkspaceContext'; +import {buildWorkspaceMocks} from '../../workspace/__fixtures__/Workspace.fixtures'; import {DUNDER_REPO_NAME} from '../../workspace/buildRepoAddress'; -import {RootWorkspaceQuery} from '../../workspace/types/WorkspaceContext.types'; const buildRepo = ({ name, @@ -38,136 +35,66 @@ const buildRepo = ({ }); }; -export const buildWorkspaceQueryWithZeroLocations = (): MockedResponse => { - return { - request: { - query: ROOT_WORKSPACE_QUERY, - variables: {}, - }, - result: { - data: { - __typename: 'Query', - workspaceOrError: buildWorkspace({ - locationEntries: [], - }), - }, - }, - }; -}; +export const buildWorkspaceQueryWithZeroLocations = () => buildWorkspaceMocks([]); -export const buildWorkspaceQueryWithOneLocation = (): MockedResponse => { - return { - request: { - query: ROOT_WORKSPACE_QUERY, - variables: {}, - }, - result: { - data: { - __typename: 'Query', - workspaceOrError: buildWorkspace({ - locationEntries: [ - buildWorkspaceLocationEntry({ - id: 'ipsum-entry', - name: 'ipsum-entry', - locationOrLoadError: buildRepositoryLocation({ - id: 'ipsum', - name: 'ipsum', - repositories: [ - buildRepo({name: 'lorem', jobNames: ['my_pipeline', 'other_pipeline']}), - ], - }), - }), - ], +const locationEntries = [ + buildWorkspaceLocationEntry({ + id: 'ipsum-entry', + name: 'ipsum-entry', + locationOrLoadError: buildRepositoryLocation({ + id: 'ipsum', + name: 'ipsum', + repositories: [buildRepo({name: 'lorem', jobNames: ['my_pipeline', 'other_pipeline']})], + }), + }), + buildWorkspaceLocationEntry({ + id: 'bar-entry', + name: 'bar-entry', + locationOrLoadError: buildRepositoryLocation({ + id: 'bar', + name: 'bar', + repositories: [buildRepo({name: 'foo', jobNames: ['bar_job', 'd_job', 'e_job', 'f_job']})], + }), + }), + buildWorkspaceLocationEntry({ + id: 'abc_location-entry', + name: 'abc_location-entry', + locationOrLoadError: buildRepositoryLocation({ + id: 'abc_location', + name: 'abc_location', + repositories: [ + buildRepo({ + name: DUNDER_REPO_NAME, + jobNames: ['abc_job', 'def_job', 'ghi_job', 'jkl_job', 'mno_job', 'pqr_job'], }), - }, - }, - }; + ], + }), + }), +] as [WorkspaceLocationEntry, WorkspaceLocationEntry, WorkspaceLocationEntry]; + +export const buildWorkspaceQueryWithOneLocation = () => { + return buildWorkspaceMocks([locationEntries[0]]); }; -export const buildWorkspaceQueryWithThreeLocations = (): MockedResponse => { - return { - request: { - query: ROOT_WORKSPACE_QUERY, - variables: {}, - }, - result: { - data: { - __typename: 'Query', - workspaceOrError: buildWorkspace({ - locationEntries: [ - buildWorkspaceLocationEntry({ - id: 'ipsum-entry', - name: 'ipsum-entry', - locationOrLoadError: buildRepositoryLocation({ - id: 'ipsum', - name: 'ipsum', - repositories: [ - buildRepo({name: 'lorem', jobNames: ['my_pipeline', 'other_pipeline']}), - ], - }), - }), - buildWorkspaceLocationEntry({ - id: 'bar-entry', - name: 'bar-entry', - locationOrLoadError: buildRepositoryLocation({ - id: 'bar', - name: 'bar', - repositories: [ - buildRepo({name: 'foo', jobNames: ['bar_job', 'd_job', 'e_job', 'f_job']}), - ], - }), - }), - buildWorkspaceLocationEntry({ - id: 'abc_location-entry', - name: 'abc_location-entry', - locationOrLoadError: buildRepositoryLocation({ - id: 'abc_location', - name: 'abc_location', - repositories: [ - buildRepo({ - name: DUNDER_REPO_NAME, - jobNames: ['abc_job', 'def_job', 'ghi_job', 'jkl_job', 'mno_job', 'pqr_job'], - }), - ], - }), - }), - ], - }), - }, - }, - }; +export const buildWorkspaceQueryWithThreeLocations = () => { + return buildWorkspaceMocks(locationEntries); }; -export const buildWorkspaceQueryWithOneLocationAndAssetGroup = - (): MockedResponse => { - return { - request: { - query: ROOT_WORKSPACE_QUERY, - variables: {}, - }, - result: { - data: { - __typename: 'Query', - workspaceOrError: buildWorkspace({ - locationEntries: [ - buildWorkspaceLocationEntry({ - id: 'ipsum-entry', - name: 'ipsum-entry', - locationOrLoadError: buildRepositoryLocation({ - id: 'ipsum', - name: 'ipsum', - repositories: [ - buildRepo({ - name: 'lorem', - jobNames: ['my_pipeline', 'other_pipeline'], - assetGroupNames: ['my_asset_group'], - }), - ], - }), - }), - ], - }), - }, - }, - }; - }; +const entryWithOneLocationAndAssetGroup = buildWorkspaceLocationEntry({ + id: 'unique-entry', + name: 'unique-entry', + locationOrLoadError: buildRepositoryLocation({ + id: 'unique', + name: 'unique', + repositories: [ + buildRepo({ + name: 'entry', + jobNames: ['my_pipeline', 'other_pipeline'], + assetGroupNames: ['my_asset_group'], + }), + ], + }), +}); + +export const buildWorkspaceQueryWithOneLocationAndAssetGroup = () => + buildWorkspaceMocks([entryWithOneLocationAndAssetGroup]); diff --git a/js_modules/dagster-ui/packages/ui-core/src/nav/__fixtures__/useDaemonStatus.fixtures.tsx b/js_modules/dagster-ui/packages/ui-core/src/nav/__fixtures__/useDaemonStatus.fixtures.tsx index 37fb0d1bbde44..daabb6cd721c0 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/nav/__fixtures__/useDaemonStatus.fixtures.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/nav/__fixtures__/useDaemonStatus.fixtures.tsx @@ -12,13 +12,11 @@ import { buildRepositoryLocation, buildSchedule, buildSensor, - buildWorkspace, buildWorkspaceLocationEntry, } from '../../graphql/types'; import {InstanceWarningQuery} from '../../instance/types/useDaemonStatus.types'; import {INSTANCE_WARNING_QUERY} from '../../instance/useDaemonStatus'; -import {ROOT_WORKSPACE_QUERY} from '../../workspace/WorkspaceContext'; -import {RootWorkspaceQuery} from '../../workspace/types/WorkspaceContext.types'; +import {buildWorkspaceMocks} from '../../workspace/__fixtures__/Workspace.fixtures'; const buildRepo = ({ name, @@ -55,33 +53,18 @@ const buildRepo = ({ }); }; -export const buildWorkspaceQueryWithNoSchedulesOrSensors = - (): MockedResponse => { - return { - request: { - query: ROOT_WORKSPACE_QUERY, - variables: {}, - }, - result: { - data: { - __typename: 'Query', - workspaceOrError: buildWorkspace({ - locationEntries: [ - buildWorkspaceLocationEntry({ - id: 'ipsum-entry', - name: 'ipsum-entry', - locationOrLoadError: buildRepositoryLocation({ - id: 'ipsum', - name: 'ipsum', - repositories: [buildRepo({name: 'lorem'})], - }), - }), - ], - }), - }, - }, - }; - }; +export const buildWorkspaceQueryWithNoSchedulesOrSensors = () => + buildWorkspaceMocks([ + buildWorkspaceLocationEntry({ + id: 'ipsum-entry', + name: 'ipsum-entry', + locationOrLoadError: buildRepositoryLocation({ + id: 'ipsum', + name: 'ipsum', + repositories: [buildRepo({name: 'lorem'})], + }), + }), + ]); export const buildWorkspaceQueryWithScheduleAndSensor = ({ schedule, @@ -89,37 +72,24 @@ export const buildWorkspaceQueryWithScheduleAndSensor = ({ }: { schedule: InstigationStatus; sensor: InstigationStatus; -}): MockedResponse => { - return { - request: { - query: ROOT_WORKSPACE_QUERY, - variables: {}, - }, - result: { - data: { - __typename: 'Query', - workspaceOrError: buildWorkspace({ - locationEntries: [ - buildWorkspaceLocationEntry({ - id: 'ipsum-entry', - name: 'ipsum-entry', - locationOrLoadError: buildRepositoryLocation({ - id: 'ipsum', - name: 'ipsum', - repositories: [ - buildRepo({ - name: 'lorem', - schedules: {'my-schedule': schedule}, - sensors: {'my-sensor': sensor}, - }), - ], - }), - }), - ], - }), - }, - }, - }; +}) => { + return buildWorkspaceMocks([ + buildWorkspaceLocationEntry({ + id: 'ipsum-entry' + Math.random(), + name: 'ipsum-entry' + Math.random(), + locationOrLoadError: buildRepositoryLocation({ + id: 'ipsum', + name: 'ipsum', + repositories: [ + buildRepo({ + name: 'lorem', + schedules: {'my-schedule': schedule}, + sensors: {'my-sensor': sensor}, + }), + ], + }), + }), + ]); }; type DaemonHealth = {daemonType: string; healthy: boolean; required: boolean}[]; diff --git a/js_modules/dagster-ui/packages/ui-core/src/nav/__tests__/LeftNavRepositorySection.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/nav/__tests__/LeftNavRepositorySection.test.tsx index 39bd0ab452211..83b411950e634 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/nav/__tests__/LeftNavRepositorySection.test.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/nav/__tests__/LeftNavRepositorySection.test.tsx @@ -29,27 +29,27 @@ describe('Repository options', () => { let nativeGBRC: any; - beforeAll(() => { + beforeEach(() => { + window.localStorage.clear(); nativeGBRC = window.Element.prototype.getBoundingClientRect; window.Element.prototype.getBoundingClientRect = jest .fn() .mockReturnValue({height: 400, width: 400}); }); - afterAll(() => { - window.Element.prototype.getBoundingClientRect = nativeGBRC; - }); - afterEach(() => { + window.Element.prototype.getBoundingClientRect = nativeGBRC; window.localStorage.clear(); __resetForJest(); + jest.resetModules(); + jest.resetAllMocks(); }); it('Correctly displays the current repository state', async () => { await act(() => render( - + @@ -71,7 +71,7 @@ describe('Repository options', () => { await act(() => render( - + @@ -94,7 +94,7 @@ describe('Repository options', () => { await act(() => render( - + @@ -116,7 +116,7 @@ describe('Repository options', () => { await act(() => render( - + @@ -127,18 +127,14 @@ describe('Repository options', () => { const loremHeader = await screen.findByRole('button', {name: /lorem/i}); expect(loremHeader).toBeVisible(); - const fooHeader = screen.getByRole('button', {name: /foo/i}); + const fooHeader = await waitFor(() => screen.getByRole('button', {name: /foo/i})); expect(fooHeader).toBeVisible(); - const dunderHeader = screen.getByRole('button', {name: /abc_location/i}); + const dunderHeader = await waitFor(() => screen.getByRole('button', {name: /abc_location/i})); expect(dunderHeader).toBeVisible(); await userEvent.click(loremHeader); - await userEvent.click(fooHeader); - await userEvent.click(dunderHeader); - await waitFor(() => { - // Twelve jobs total. No repo name link since multiple repos are visible. - expect(screen.queryAllByRole('link')).toHaveLength(12); + expect(screen.queryAllByRole('link')).toHaveLength(6); }); }); @@ -151,7 +147,7 @@ describe('Repository options', () => { await act(() => render( - + @@ -175,7 +171,7 @@ describe('Repository options', () => { await act(() => render( - + @@ -214,7 +210,7 @@ describe('Repository options', () => { act(() => { render( - + @@ -247,8 +243,8 @@ describe('Repository options', () => { }; const mocks = [ - buildWorkspaceQueryWithZeroLocations(), - buildWorkspaceQueryWithThreeLocations(), + ...buildWorkspaceQueryWithZeroLocations(), + ...buildWorkspaceQueryWithThreeLocations(), ]; await act(() => @@ -269,7 +265,7 @@ describe('Repository options', () => { const reloadButton = screen.getByRole('button', {name: /refetch workspace/i}); await userEvent.click(reloadButton); - const loremHeader = await screen.findByRole('button', {name: /lorem/i}); + const loremHeader = await waitFor(() => screen.findByRole('button', {name: /lorem/i})); await waitFor(() => { expect(loremHeader).toBeVisible(); }); @@ -303,7 +299,10 @@ describe('Repository options', () => { ); }; - const mocks = [buildWorkspaceQueryWithOneLocation(), buildWorkspaceQueryWithZeroLocations()]; + const mocks = [ + ...buildWorkspaceQueryWithOneLocation(), + ...buildWorkspaceQueryWithZeroLocations(), + ]; await act(() => render( @@ -339,7 +338,7 @@ describe('Repository options', () => { await act(() => render( - + @@ -348,7 +347,7 @@ describe('Repository options', () => { ), ); - const repoHeader = await screen.findByRole('button', {name: /lorem/i}); + const repoHeader = await screen.findByRole('button', {name: /unique/i}); await userEvent.click(repoHeader); await waitFor(() => { diff --git a/js_modules/dagster-ui/packages/ui-core/src/nav/__tests__/useDaemonStatus.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/nav/__tests__/useDaemonStatus.test.tsx index d93b970707fef..f143ace001c66 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/nav/__tests__/useDaemonStatus.test.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/nav/__tests__/useDaemonStatus.test.tsx @@ -1,10 +1,10 @@ import {MockedProvider} from '@apollo/client/testing'; -import {render, screen} from '@testing-library/react'; -import {renderHook} from '@testing-library/react-hooks'; +import {render, renderHook, screen, waitFor} from '@testing-library/react'; import * as React from 'react'; import {InstigationStatus} from '../../graphql/types'; import {useDaemonStatus} from '../../instance/useDaemonStatus'; +import {__resetForJest} from '../../search/useIndexedDBCachedQuery'; import {WorkspaceProvider} from '../../workspace/WorkspaceContext'; import { buildInstanceWarningQuery, @@ -12,15 +12,18 @@ import { buildWorkspaceQueryWithScheduleAndSensor, } from '../__fixtures__/useDaemonStatus.fixtures'; +afterEach(() => { + __resetForJest(); +}); describe('useDaemonStatus', () => { describe('Scheduler daemon', () => { it('does not surface scheduler errors if there are no schedules', async () => { const daemonHealth = [{daemonType: 'SCHEDULER', healthy: false, required: true}]; - const {result, waitForNextUpdate} = renderHook(() => useDaemonStatus(), { + const {result} = renderHook(() => useDaemonStatus(), { wrapper: ({children}: {children: React.ReactNode}) => ( @@ -29,18 +32,17 @@ describe('useDaemonStatus', () => { ), }); - await waitForNextUpdate(); expect(result.current).toBeNull(); }); it('does not surface scheduler errors if there are no running schedules', async () => { const daemonHealth = [{daemonType: 'SCHEDULER', healthy: false, required: true}]; - const {result, waitForNextUpdate} = renderHook(() => useDaemonStatus(), { + const {result} = renderHook(() => useDaemonStatus(), { wrapper: ({children}: {children: React.ReactNode}) => ( { ), }); - await waitForNextUpdate(); expect(result.current).toBeNull(); }); it('does surface scheduler errors if there is a running schedule', async () => { const daemonHealth = [{daemonType: 'SCHEDULER', healthy: false, required: true}]; - const {result, waitFor} = renderHook(() => useDaemonStatus(), { + const {result} = renderHook(() => useDaemonStatus(), { wrapper: ({children}: {children: React.ReactNode}) => ( { it('does not surface sensor daemon errors if there are no sensors', async () => { const daemonHealth = [{daemonType: 'SENSOR', healthy: false, required: true}]; - const {result, waitForNextUpdate} = renderHook(() => useDaemonStatus(), { + const {result} = renderHook(() => useDaemonStatus(), { wrapper: ({children}: {children: React.ReactNode}) => ( @@ -100,18 +101,17 @@ describe('useDaemonStatus', () => { ), }); - await waitForNextUpdate(); expect(result.current).toBeNull(); }); it('does not surface sensor daemon errors if there are no running sensors', async () => { const daemonHealth = [{daemonType: 'SENSOR', healthy: false, required: true}]; - const {result, waitForNextUpdate} = renderHook(() => useDaemonStatus(), { + const {result} = renderHook(() => useDaemonStatus(), { wrapper: ({children}: {children: React.ReactNode}) => ( { ), }); - await waitForNextUpdate(); expect(result.current).toBeNull(); }); it('does surface sensor daemon errors if there is a running sensor', async () => { const daemonHealth = [{daemonType: 'SENSOR', healthy: false, required: true}]; - const {result, waitFor} = renderHook(() => useDaemonStatus(), { + const {result} = renderHook(() => useDaemonStatus(), { wrapper: ({children}: {children: React.ReactNode}) => ( { it('does not surface backfill daemon errors if there are no backfills', async () => { const daemonHealth = [{daemonType: 'BACKFILL', healthy: false, required: true}]; - const {result, waitForNextUpdate} = renderHook(() => useDaemonStatus(), { + const {result} = renderHook(() => useDaemonStatus(), { wrapper: ({children}: {children: React.ReactNode}) => ( @@ -171,18 +170,17 @@ describe('useDaemonStatus', () => { ), }); - await waitForNextUpdate(); expect(result.current).toBeNull(); }); it('does surface backfill daemon errors if there is a backfill', async () => { const daemonHealth = [{daemonType: 'BACKFILL', healthy: false, required: true}]; - const {result, waitFor} = renderHook(() => useDaemonStatus(), { + const {result} = renderHook(() => useDaemonStatus(), { wrapper: ({children}: {children: React.ReactNode}) => ( @@ -207,11 +205,11 @@ describe('useDaemonStatus', () => { {daemonType: 'BACKFILL', healthy: false, required: true}, ]; - const {result, waitFor} = renderHook(() => useDaemonStatus(), { + const {result} = renderHook(() => useDaemonStatus(), { wrapper: ({children}: {children: React.ReactNode}) => ( ; - -export type CodeLocationStatusQuery = { - __typename: 'Query'; - locationStatusesOrError: - | {__typename: 'PythonError'} - | { - __typename: 'WorkspaceLocationStatusEntries'; - entries: Array<{ - __typename: 'WorkspaceLocationStatusEntry'; - id: string; - name: string; - loadStatus: Types.RepositoryLocationLoadStatus; - }>; - }; -}; diff --git a/js_modules/dagster-ui/packages/ui-core/src/nav/useCodeLocationsStatus.tsx b/js_modules/dagster-ui/packages/ui-core/src/nav/useCodeLocationsStatus.tsx index 907fd21f22570..1a3f0db69db01 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/nav/useCodeLocationsStatus.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/nav/useCodeLocationsStatus.tsx @@ -1,18 +1,14 @@ -import {gql, useQuery} from '@apollo/client'; import {Box, ButtonLink, Colors} from '@dagster-io/ui-components'; -import {useCallback, useContext, useState} from 'react'; +import {useCallback, useContext, useLayoutEffect, useState} from 'react'; import {useHistory} from 'react-router-dom'; +import {atom, useRecoilValue} from 'recoil'; import styled from 'styled-components'; -import { - CodeLocationStatusQuery, - CodeLocationStatusQueryVariables, -} from './types/useCodeLocationsStatus.types'; import {showSharedToaster} from '../app/DomUtils'; -import {useQueryRefreshAtInterval} from '../app/QueryRefresh'; import {RepositoryLocationLoadStatus} from '../graphql/types'; import {StatusAndMessage} from '../instance/DeploymentStatusType'; import {WorkspaceContext} from '../workspace/WorkspaceContext'; +import {CodeLocationStatusQuery} from '../workspace/types/WorkspaceQueries.types'; type LocationStatusEntry = { loadStatus: RepositoryLocationLoadStatus; @@ -20,12 +16,15 @@ type LocationStatusEntry = { name: string; }; -const POLL_INTERVAL = 5 * 1000; - type EntriesById = Record; -export const useCodeLocationsStatus = (skip = false): StatusAndMessage | null => { - const {locationEntries, refetch} = useContext(WorkspaceContext); +export const codeLocationStatusAtom = atom({ + key: 'codeLocationStatusQuery', + default: undefined, +}); + +export const useCodeLocationsStatus = (): StatusAndMessage | null => { + const {locationEntries, data} = useContext(WorkspaceContext); const [previousEntriesById, setPreviousEntriesById] = useState(null); const history = useHistory(); @@ -37,28 +36,19 @@ export const useCodeLocationsStatus = (skip = false): StatusAndMessage | null => }, [history]); // Reload the workspace, but don't toast about it. - const reloadWorkspaceQuietly = useCallback(async () => { - setShowSpinner(true); - await refetch(); - setShowSpinner(false); - }, [refetch]); // Reload the workspace, and show a success or error toast upon completion. - const reloadWorkspaceLoudly = useCallback(async () => { - setShowSpinner(true); - const result = await refetch(); - setShowSpinner(false); - - const anyErrors = - result.data.workspaceOrError.__typename === 'PythonError' || - result.data.workspaceOrError.locationEntries.some( - (entry) => entry.locationOrLoadError?.__typename === 'PythonError', - ); + useLayoutEffect(() => { + const anyErrors = Object.values(data).some( + (entry) => + entry.__typename === 'PythonError' || + entry.locationOrLoadError?.__typename === 'PythonError', + ); const showViewButton = !alreadyViewingCodeLocations(); if (anyErrors) { - await showSharedToaster({ + showSharedToaster({ intent: 'warning', message: ( @@ -68,31 +58,38 @@ export const useCodeLocationsStatus = (skip = false): StatusAndMessage | null => ), icon: 'check_circle', }); - } else { - await showSharedToaster({ - intent: 'success', - message: ( - -
Definitions reloaded
- {showViewButton ? : null} - - ), - icon: 'check_circle', - }); } - }, [onClickViewButton, refetch]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [data, onClickViewButton]); + + const codeLocationStatusQueryData = useRecoilValue(codeLocationStatusAtom); - const onLocationUpdate = async (data: CodeLocationStatusQuery) => { + useLayoutEffect(() => { const isFreshPageload = previousEntriesById === null; + const showViewButton = !alreadyViewingCodeLocations(); // Given the previous and current code locations, determine whether to show a) a loading spinner // and/or b) a toast indicating that a code location is being reloaded. const entries = - data?.locationStatusesOrError?.__typename === 'WorkspaceLocationStatusEntries' - ? data?.locationStatusesOrError.entries + codeLocationStatusQueryData?.locationStatusesOrError?.__typename === + 'WorkspaceLocationStatusEntries' + ? codeLocationStatusQueryData?.locationStatusesOrError.entries : []; let hasUpdatedEntries = entries.length !== Object.keys(previousEntriesById || {}).length; + + if (!isFreshPageload && hasUpdatedEntries) { + showSharedToaster({ + intent: 'success', + message: ( + +
Definitions reloaded
+ {showViewButton ? : null} + + ), + icon: 'check_circle', + }); + } const currEntriesById: {[key: string]: LocationStatusEntry} = {}; entries.forEach((entry) => { const previousEntry = previousEntriesById && previousEntriesById[entry.id]; @@ -132,12 +129,9 @@ export const useCodeLocationsStatus = (skip = false): StatusAndMessage | null => // At least one code location has been removed. Reload, but don't make a big deal about it // since this was probably done manually. if (previousEntries.length > currentEntries.length) { - reloadWorkspaceQuietly(); return; } - const showViewButton = !alreadyViewingCodeLocations(); - // We have a new entry, and it has already finished loading. Wow! It's surprisingly fast for it // to have finished loading so quickly, but go ahead and indicate that the location has // been added, then reload the workspace. @@ -168,7 +162,7 @@ export const useCodeLocationsStatus = (skip = false): StatusAndMessage | null => return {addedEntries.length} code locations added; }; - await showSharedToaster({ + showSharedToaster({ intent: 'primary', message: ( @@ -179,7 +173,6 @@ export const useCodeLocationsStatus = (skip = false): StatusAndMessage | null => icon: 'add_circle', }); - reloadWorkspaceLoudly(); return; } @@ -192,7 +185,7 @@ export const useCodeLocationsStatus = (skip = false): StatusAndMessage | null => if (!anyPreviouslyLoading && anyCurrentlyLoading) { setShowSpinner(true); - await showSharedToaster({ + showSharedToaster({ intent: 'primary', message: ( @@ -211,30 +204,8 @@ export const useCodeLocationsStatus = (skip = false): StatusAndMessage | null => return; } - - // A location was previously loading, and no longer is. Our workspace is ready. Refetch it. - if (anyPreviouslyLoading && !anyCurrentlyLoading) { - reloadWorkspaceLoudly(); - return; - } - - if (hasUpdatedEntries) { - reloadWorkspaceLoudly(); - return; - } - }; - - const queryData = useQuery( - CODE_LOCATION_STATUS_QUERY, - { - fetchPolicy: 'network-only', - notifyOnNetworkStatusChange: true, - skip, - onCompleted: onLocationUpdate, - }, - ); - - useQueryRefreshAtInterval(queryData, POLL_INTERVAL); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [codeLocationStatusQueryData]); if (showSpinner) { return { @@ -274,17 +245,3 @@ const ViewCodeLocationsButton = ({onClick}: {onClick: () => void}) => { const ViewButton = styled(ButtonLink)` white-space: nowrap; `; - -const CODE_LOCATION_STATUS_QUERY = gql` - query CodeLocationStatusQuery { - locationStatusesOrError { - ... on WorkspaceLocationStatusEntries { - entries { - id - name - loadStatus - } - } - } - } -`; diff --git a/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewSensors.tsx b/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewSensors.tsx index a7b4f3c51bd43..922220e762798 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewSensors.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/overview/OverviewSensors.tsx @@ -32,7 +32,6 @@ import {SENSOR_TYPE_META} from '../workspace/VirtualizedSensorRow'; import {WorkspaceContext} from '../workspace/WorkspaceContext'; import {buildRepoAddress} from '../workspace/buildRepoAddress'; import {repoAddressAsHumanString} from '../workspace/repoAddressAsString'; -import {RootWorkspaceQuery} from '../workspace/types/WorkspaceContext.types'; function toSetFilterValue(type: SensorType) { const label = SENSOR_TYPE_META[type].name; @@ -55,12 +54,7 @@ const SENSOR_TYPE_TO_FILTER: Partial { - const { - allRepos, - visibleRepos, - loading: workspaceLoading, - data: cachedData, - } = useContext(WorkspaceContext); + const {allRepos, visibleRepos, loading: workspaceLoading} = useContext(WorkspaceContext); const repoCount = allRepos.length; const [searchValue, setSearchValue] = useQueryPersistedState({ @@ -103,12 +97,7 @@ export const OverviewSensors = () => { notifyOnNetworkStatusChange: true, }, ); - const {data: currentData, loading} = queryResultOverview; - const data = - currentData ?? - (cachedData?.workspaceOrError.__typename === 'Workspace' - ? (cachedData as Extract) - : null); + const {data, loading} = queryResultOverview; useBlockTraceOnQueryResult(queryResultOverview, 'OverviewSensorsQuery'); @@ -340,7 +329,7 @@ export const OverviewSensors = () => { ) : ( <> @@ -351,7 +340,7 @@ export const OverviewSensors = () => { ); }; -const buildBuckets = (data?: null | OverviewSensorsQuery | RootWorkspaceQuery) => { +const buildBuckets = (data?: null | OverviewSensorsQuery) => { if (data?.workspaceOrError.__typename !== 'Workspace') { return []; } diff --git a/js_modules/dagster-ui/packages/ui-core/src/runs/HourlyDataCache/HourlyDataCache.tsx b/js_modules/dagster-ui/packages/ui-core/src/runs/HourlyDataCache/HourlyDataCache.tsx index 992aaccff860e..0995a457a79b3 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/runs/HourlyDataCache/HourlyDataCache.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/runs/HourlyDataCache/HourlyDataCache.tsx @@ -7,7 +7,7 @@ export const ONE_HOUR_S = 60 * 60; type Subscription = (data: T[]) => void; export const defaultOptions = { - expiry: new Date('3000-01-01'), // never expire, + expiry: new Date('3030-01-01'), // never expire, }; export class HourlyDataCache { diff --git a/js_modules/dagster-ui/packages/ui-core/src/runs/__fixtures__/RunActionsMenu.fixtures.tsx b/js_modules/dagster-ui/packages/ui-core/src/runs/__fixtures__/RunActionsMenu.fixtures.tsx index e0c89d4cccf42..1ba3297465285 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/runs/__fixtures__/RunActionsMenu.fixtures.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/runs/__fixtures__/RunActionsMenu.fixtures.tsx @@ -7,11 +7,9 @@ import { buildRepositoryLocation, buildRepositoryOrigin, buildRun, - buildWorkspace, buildWorkspaceLocationEntry, } from '../../graphql/types'; -import {ROOT_WORKSPACE_QUERY} from '../../workspace/WorkspaceContext'; -import {RootWorkspaceQuery} from '../../workspace/types/WorkspaceContext.types'; +import {buildWorkspaceMocks} from '../../workspace/__fixtures__/Workspace.fixtures'; import {PIPELINE_ENVIRONMENT_QUERY} from '../RunActionsMenu'; import { PipelineEnvironmentQuery, @@ -57,41 +55,28 @@ export const buildRunActionsMenuFragment = ({hasReExecutePermission}: RunConfigI }); }; -export const buildMockRootWorkspaceQuery = (): MockedResponse => { - return { - request: { - query: ROOT_WORKSPACE_QUERY, - }, - result: { - data: { - __typename: 'Query', - workspaceOrError: buildWorkspace({ - id: 'workspace', - locationEntries: [ - buildWorkspaceLocationEntry({ - id: LOCATION_NAME, - locationOrLoadError: buildRepositoryLocation({ - id: LOCATION_NAME, - repositories: [ - buildRepository({ - id: REPO_NAME, - name: REPO_NAME, - pipelines: [ - buildPipeline({ - id: JOB_NAME, - name: JOB_NAME, - pipelineSnapshotId: SNAPSHOT_ID, - }), - ], - }), - ], +export const buildMockRootWorkspaceQuery = () => { + return buildWorkspaceMocks([ + buildWorkspaceLocationEntry({ + id: LOCATION_NAME, + locationOrLoadError: buildRepositoryLocation({ + id: LOCATION_NAME, + repositories: [ + buildRepository({ + id: REPO_NAME, + name: REPO_NAME, + pipelines: [ + buildPipeline({ + id: JOB_NAME, + name: JOB_NAME, + pipelineSnapshotId: SNAPSHOT_ID, }), - }), - ], - }), - }, - }, - }; + ], + }), + ], + }), + }), + ]); }; export const buildPipelineEnvironmentQuery = ( diff --git a/js_modules/dagster-ui/packages/ui-core/src/runs/__fixtures__/RunsFilterInput.fixtures.tsx b/js_modules/dagster-ui/packages/ui-core/src/runs/__fixtures__/RunsFilterInput.fixtures.tsx index c9691a984b294..86ef39f9f01d8 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/runs/__fixtures__/RunsFilterInput.fixtures.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/runs/__fixtures__/RunsFilterInput.fixtures.tsx @@ -1,26 +1,10 @@ import {MockedResponse} from '@apollo/client/testing'; -import {WorkspaceOrError, buildPipelineTagAndValues, buildRunTags} from '../../graphql/types'; -import {ROOT_WORKSPACE_QUERY} from '../../workspace/WorkspaceContext'; -import {RootWorkspaceQuery} from '../../workspace/types/WorkspaceContext.types'; +import {buildPipelineTagAndValues, buildRunTags} from '../../graphql/types'; import {DagsterTag} from '../RunTag'; import {RUN_TAG_VALUES_QUERY} from '../RunsFilterInput'; import {RunTagValuesQuery} from '../types/RunsFilterInput.types'; -export const buildWorkspaceContextMockedResponse = ( - workspaceOrError: WorkspaceOrError, -): MockedResponse => ({ - request: { - query: ROOT_WORKSPACE_QUERY, - }, - result: { - data: { - __typename: 'Query', - workspaceOrError, - }, - }, -}); - export function buildRunTagValuesQueryMockedResponse( tagKey: DagsterTag, values: string[], diff --git a/js_modules/dagster-ui/packages/ui-core/src/runs/__tests__/RunActionButtons.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/runs/__tests__/RunActionButtons.test.tsx index f70825b651d47..ae9a8752ee3fc 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/runs/__tests__/RunActionButtons.test.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/runs/__tests__/RunActionButtons.test.tsx @@ -26,7 +26,7 @@ describe('RunActionButtons', () => { const Test = ({run}: {run: RunPageFragment}) => { return ( - + diff --git a/js_modules/dagster-ui/packages/ui-core/src/runs/__tests__/RunActionsMenu.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/runs/__tests__/RunActionsMenu.test.tsx index 48e0c83b49c39..b171bc746a8fa 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/runs/__tests__/RunActionsMenu.test.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/runs/__tests__/RunActionsMenu.test.tsx @@ -17,7 +17,7 @@ describe('RunActionsMenu', () => { render( @@ -45,7 +45,7 @@ describe('RunActionsMenu', () => { render( diff --git a/js_modules/dagster-ui/packages/ui-core/src/runs/__tests__/RunFilterInput.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/runs/__tests__/RunFilterInput.test.tsx index 49124ba600e2c..6758f47510328 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/runs/__tests__/RunFilterInput.test.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/runs/__tests__/RunFilterInput.test.tsx @@ -9,11 +9,11 @@ import { buildRepository, buildRepositoryLocation, buildRunTagKeys, - buildWorkspace, buildWorkspaceLocationEntry, } from '../../graphql/types'; import {calculateTimeRanges} from '../../ui/Filters/useTimeRangeFilter'; import {WorkspaceProvider} from '../../workspace/WorkspaceContext'; +import {buildWorkspaceMocks} from '../../workspace/__fixtures__/Workspace.fixtures'; import {DagsterTag} from '../RunTag'; import { RUN_TAG_KEYS_QUERY, @@ -24,34 +24,27 @@ import { useRunsFilterInput, useTagDataFilterValues, } from '../RunsFilterInput'; -import { - buildRunTagValuesQueryMockedResponse, - buildWorkspaceContextMockedResponse, -} from '../__fixtures__/RunsFilterInput.fixtures'; +import {buildRunTagValuesQueryMockedResponse} from '../__fixtures__/RunsFilterInput.fixtures'; import {RunTagKeysQuery} from '../types/RunsFilterInput.types'; -const workspaceMock = buildWorkspaceContextMockedResponse( - buildWorkspace({ - locationEntries: [ - buildWorkspaceLocationEntry({ - name: 'some_workspace', - locationOrLoadError: buildRepositoryLocation({ - name: 'some_location', - repositories: [ - buildRepository({ - name: 'some_repo', - pipelines: [ - buildPipeline({ - name: 'some_job', - }), - ], +const workspaceMocks = buildWorkspaceMocks([ + buildWorkspaceLocationEntry({ + name: 'some_workspace', + locationOrLoadError: buildRepositoryLocation({ + name: 'some_location', + repositories: [ + buildRepository({ + name: 'some_repo', + pipelines: [ + buildPipeline({ + name: 'some_job', }), ], }), - }), - ], + ], + }), }), -); +]); const runTagKeysMock: MockedResponse = { request: { @@ -168,7 +161,7 @@ function TestRunsFilterInput({ ); } return ( - + @@ -219,7 +212,7 @@ describe('', () => { tokens={tokens} onChange={onChange} enabledFilters={['job']} - mocks={[workspaceMock]} + mocks={workspaceMocks} />, ); diff --git a/js_modules/dagster-ui/packages/ui-core/src/search/__tests__/useIndexedDBCachedQuery.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/search/__tests__/useIndexedDBCachedQuery.test.tsx index 2529134c9ab4d..747099649b26b 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/search/__tests__/useIndexedDBCachedQuery.test.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/search/__tests__/useIndexedDBCachedQuery.test.tsx @@ -24,6 +24,7 @@ jest.mock('idb-lru-cache', () => { has: jest.fn(), get: jest.fn(), set: jest.fn(), + delete: jest.fn(), }; return { cache: jest.fn(() => mockedCache), diff --git a/js_modules/dagster-ui/packages/ui-core/src/search/useIndexedDBCachedQuery.tsx b/js_modules/dagster-ui/packages/ui-core/src/search/useIndexedDBCachedQuery.tsx index eaab4a86bb66d..16d016d6b9804 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/search/useIndexedDBCachedQuery.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/search/useIndexedDBCachedQuery.tsx @@ -1,118 +1,195 @@ -import {ApolloQueryResult, DocumentNode, OperationVariables, useApolloClient} from '@apollo/client'; +import {ApolloClient, DocumentNode, OperationVariables, useApolloClient} from '@apollo/client'; import {cache} from 'idb-lru-cache'; -import React from 'react'; +import memoize from 'lodash/memoize'; +import React, {createContext, useCallback, useContext} from 'react'; type CacheData = { data: TQuery; version: number; }; -let fetchState: Record< - string, - { - onFetched: ((value: any) => void)[]; +export const KEY_PREFIX = 'indexdbQueryCache:'; + +export class CacheManager { + private cache: ReturnType>>; + private key: string; + + constructor(key: string) { + this.key = `${KEY_PREFIX}${key}`; + this.cache = cache>({dbName: this.key, maxCount: 1}); + } + + async get(version: number): Promise { + if (await this.cache.has('cache')) { + const {value} = await this.cache.get('cache'); + if (value && version === value.version) { + return value.data; + } + } + return null; + } + + async set(data: TQuery, version: number): Promise { + return this.cache.set('cache', {data, version}, {expiry: new Date('3030-01-01')}); + } + + async clear() { + await this.cache.delete('cache'); } -> = {}; +} + +interface QueryHookParams { + key: string; + query: DocumentNode; + version: number; + variables?: TVariables; + onCompleted?: (data: TQuery) => void; +} -/** - * Returns data from the indexedDB cache initially while loading is true. - * Fetches data from the network/cache initially and does not receive any updates afterwards - * Uses fetch-policy: no-cache to avoid slow apollo cache normalization - */ export function useIndexedDBCachedQuery({ key, query, version, variables, -}: { - key: string; - query: DocumentNode; - version: number; - variables?: TVariables; -}) { +}: QueryHookParams) { const client = useApolloClient(); + const [data, setData] = React.useState(null); + const [loading, setLoading] = React.useState(true); + + const getData = useGetData(); - const lru = React.useMemo( - () => cache>({dbName: `indexdbQueryCache:${key}`, maxCount: 1}), - [key], + const fetch = useCallback( + async (bypassCache = false) => { + setLoading(true); + const newData = await getData({ + client, + key, + query, + variables, + version, + bypassCache, + }); + setData(newData); + setLoading(false); + }, + [getData, client, key, query, variables, version], ); - const [data, setData] = React.useState(null); + React.useEffect(() => { + fetch(); + }, [fetch]); - const [loading, setLoading] = React.useState(false); - const [cacheBreaker, setCacheBreaker] = React.useState(0); + return { + data, + loading, + fetch: useCallback(() => fetch(true), [fetch]), + }; +} - React.useEffect(() => { - (async () => { - if (await lru.has('cache')) { - const {value} = await lru.get('cache'); - if (value) { - if (version === (value.version || null)) { - setData(value.data); - } +interface FetchParams { + client: ApolloClient; + key: string; + query: DocumentNode; + variables?: TVariables; + version: number; + bypassCache?: boolean; +} + +export function useGetData() { + const {getCacheManager, fetchState} = useContext(IndexedDBCacheContext); + + return useCallback( + async ({ + client, + key, + query, + variables, + version, + bypassCache = false, + }: FetchParams): Promise => { + const cacheManager = getCacheManager(key); + + if (!bypassCache) { + const cachedData = await cacheManager.get(version); + if (cachedData !== null) { + return cachedData; } } - })(); - }, [lru, version, cacheBreaker]); - - const fetch = React.useCallback(async () => { - setLoading(true); - if (fetchState[key]) { - return await new Promise>((res) => { - fetchState[key]?.onFetched.push((value) => { - setCacheBreaker((v) => v + 1); - res(value); + + const currentState = fetchState[key]; + // Handle concurrent fetch requests + if (currentState) { + return new Promise((resolve) => { + currentState!.onFetched.push(resolve as any); }); + } + + const state = {onFetched: [] as ((value: any) => void)[]}; + fetchState[key] = state; + + const queryResult = await client.query({ + query, + variables, + fetchPolicy: 'no-cache', }); - } - fetchState[key] = {onFetched: []}; - // Use client.query here so that we initially use the apollo cache if any data is available in it - // and so that we don't subscribe to any updates to that cache (useLazyQuery and useQuery would both subscribe to updates to the - // cache which can be very slow) - const queryResult = await client.query({ - query, - variables, - fetchPolicy: 'no-cache', // Don't store the result in the cache, - // should help avoid page stuttering due to granular updates to the data - }); - const {data} = queryResult; - setLoading(false); - lru.set( - 'cache', - {data, version}, - { - expiry: new Date('3000-01-01'), // never expire, - }, - ); - delete fetchState[key]; - const onFetched = fetchState[key]?.onFetched; - try { - setData(data); - } catch (e) { - setTimeout(() => { - throw e; - }); - } - onFetched?.forEach((cb) => { - try { - cb(queryResult); - } catch (e) { - setTimeout(() => { - throw e; - }); + + const {data} = queryResult; + await cacheManager.set(data, version); + + const onFetchedHandlers = state.onFetched; + if (fetchState[key] === state) { + delete fetchState[key]; // Clean up fetch state after handling } - }); - return queryResult; - }, [client, key, lru, query, variables, version]); + onFetchedHandlers.forEach((handler) => handler(data)); // Notify all waiting fetches + return data; + }, + // eslint-disable-next-line react-hooks/exhaustive-deps + [], + ); +} + +export function useGetCachedData() { + const {getCacheManager} = useContext(IndexedDBCacheContext); + + return useCallback( + async ({key, version}: {key: string; version: number}) => { + const cacheManager = getCacheManager(key); + return await cacheManager.get(version); + }, + // eslint-disable-next-line react-hooks/exhaustive-deps + [], + ); +} +export function useClearCachedData() { + const {getCacheManager} = useContext(IndexedDBCacheContext); + return useCallback( + async ({key}: {key: string}) => { + const cacheManager = getCacheManager(key); + await cacheManager.clear(); + }, + [getCacheManager], + ); +} + +const contextValue = createIndexedDBCacheContextValue(); +export const IndexedDBCacheContext = createContext(contextValue); + +export function createIndexedDBCacheContextValue() { return { - fetch, - data, - loading, + getCacheManager: memoize((key: string) => { + return new CacheManager(key); + }), + fetchState: {} as Record< + string, + { + onFetched: ((value: any) => void)[]; + } + >, }; } export const __resetForJest = () => { - fetchState = {}; + Object.assign(contextValue, createIndexedDBCacheContextValue()); }; diff --git a/js_modules/dagster-ui/packages/ui-core/src/workspace/CodeLocationRowSet.tsx b/js_modules/dagster-ui/packages/ui-core/src/workspace/CodeLocationRowSet.tsx index 84c4a300e8935..0f684461839a6 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/workspace/CodeLocationRowSet.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/workspace/CodeLocationRowSet.tsx @@ -4,23 +4,16 @@ import { ButtonLink, Colors, Icon, - JoinedButtons, MiddleTruncate, Tag, Tooltip, } from '@dagster-io/ui-components'; import {useCallback, useMemo, useState} from 'react'; -import {Link} from 'react-router-dom'; import styled from 'styled-components'; -import {CodeLocationMenu} from './CodeLocationMenu'; -import {RepositoryCountTags} from './RepositoryCountTags'; import {RepositoryLocationNonBlockingErrorDialog} from './RepositoryLocationErrorDialog'; import {WorkspaceRepositoryLocationNode} from './WorkspaceContext'; -import {buildRepoAddress} from './buildRepoAddress'; -import {repoAddressAsHumanString} from './repoAddressAsString'; -import {WorkspaceDisplayMetadataFragment} from './types/WorkspaceContext.types'; -import {workspacePathFromAddress} from './workspacePath'; +import {WorkspaceDisplayMetadataFragment} from './types/WorkspaceQueries.types'; import {showSharedToaster} from '../app/DomUtils'; import {useCopyToClipboard} from '../app/browser'; import { @@ -31,81 +24,6 @@ import { buildReloadFnForLocation, useRepositoryLocationReload, } from '../nav/useRepositoryLocationReload'; -import {TimeFromNow} from '../ui/TimeFromNow'; - -interface Props { - locationNode: WorkspaceRepositoryLocationNode; -} - -export const CodeLocationRowSet = ({locationNode}: Props) => { - const {name, locationOrLoadError} = locationNode; - - if (!locationOrLoadError || locationOrLoadError?.__typename === 'PythonError') { - return ( - - - - - - - - - - - {'\u2013'} - - - - - - - - ); - } - - const repositories = [...locationOrLoadError.repositories].sort((a, b) => - a.name.localeCompare(b.name), - ); - - return ( - <> - {repositories.map((repository) => { - const repoAddress = buildRepoAddress(repository.name, name); - const allMetadata = [...locationNode.displayMetadata, ...repository.displayMetadata]; - return ( - - - -
- - - -
- - - - - - - - - - - - - - - - - - - - - ); - })} - - ); -}; export const ImageName = ({metadata}: {metadata: WorkspaceDisplayMetadataFragment[]}) => { const copy = useCopyToClipboard(); diff --git a/js_modules/dagster-ui/packages/ui-core/src/workspace/VirtualizedCodeLocationRow.tsx b/js_modules/dagster-ui/packages/ui-core/src/workspace/VirtualizedCodeLocationRow.tsx index 7b3b8cbc90ac6..109e95b561d7a 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/workspace/VirtualizedCodeLocationRow.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/workspace/VirtualizedCodeLocationRow.tsx @@ -12,7 +12,7 @@ import {repoAddressAsHumanString} from './repoAddressAsString'; import { WorkspaceLocationNodeFragment, WorkspaceRepositoryFragment, -} from './types/WorkspaceContext.types'; +} from './types/WorkspaceQueries.types'; import {workspacePathFromAddress} from './workspacePath'; import {TimeFromNow} from '../ui/TimeFromNow'; import {HeaderCell, HeaderRow, RowCell} from '../ui/VirtualizedTable'; diff --git a/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceContext.tsx b/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceContext.tsx index 89f3b359a3d0a..57c69bd6be8c0 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceContext.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceContext.tsx @@ -1,30 +1,40 @@ -import {ApolloQueryResult, gql} from '@apollo/client'; +import {useApolloClient} from '@apollo/client'; import sortBy from 'lodash/sortBy'; -import * as React from 'react'; -import {useContext, useMemo} from 'react'; +import React, {useCallback, useContext, useLayoutEffect, useMemo, useRef, useState} from 'react'; +import {useSetRecoilState} from 'recoil'; -import {REPOSITORY_INFO_FRAGMENT} from './RepositoryInformation'; +import {CODE_LOCATION_STATUS_QUERY, LOCATION_WORKSPACE_QUERY} from './WorkspaceQueries'; import {buildRepoAddress} from './buildRepoAddress'; import {findRepoContainingPipeline} from './findRepoContainingPipeline'; import {RepoAddress} from './types'; import { - RootWorkspaceQuery, - RootWorkspaceQueryVariables, + CodeLocationStatusQuery, + CodeLocationStatusQueryVariables, + LocationWorkspaceQuery, + LocationWorkspaceQueryVariables, WorkspaceLocationFragment, WorkspaceLocationNodeFragment, WorkspaceRepositoryFragment, WorkspaceScheduleFragment, WorkspaceSensorFragment, -} from './types/WorkspaceContext.types'; +} from './types/WorkspaceQueries.types'; import {AppContext} from '../app/AppContext'; -import {PYTHON_ERROR_FRAGMENT} from '../app/PythonErrorFragment'; +import {useRefreshAtInterval} from '../app/QueryRefresh'; import {PythonErrorFragment} from '../app/types/PythonErrorFragment.types'; import {PipelineSelector} from '../graphql/types'; import {useStateWithStorage} from '../hooks/useStateWithStorage'; -import {BASIC_INSTIGATION_STATE_FRAGMENT} from '../overview/BasicInstigationStateFragment'; -import {useIndexedDBCachedQuery} from '../search/useIndexedDBCachedQuery'; -import {SENSOR_SWITCH_FRAGMENT} from '../sensors/SensorSwitch'; - +import {useUpdatingRef} from '../hooks/useUpdatingRef'; +import {codeLocationStatusAtom} from '../nav/useCodeLocationsStatus'; +import { + useClearCachedData, + useGetCachedData, + useGetData, + useIndexedDBCachedQuery, +} from '../search/useIndexedDBCachedQuery'; + +export const CODE_LOCATION_STATUS_QUERY_KEY = '/CodeLocationStatusQuery'; +export const CODE_LOCATION_STATUS_QUERY_VERSION = 1; +export const LOCATION_WORKSPACE_QUERY_VERSION = 1; type Repository = WorkspaceRepositoryFragment; type RepositoryLocation = WorkspaceLocationFragment; @@ -40,14 +50,13 @@ export interface DagsterRepoOption { type SetVisibleOrHiddenFn = (repoAddresses: RepoAddress[]) => void; type WorkspaceState = { - error: PythonErrorFragment | null; loading: boolean; locationEntries: WorkspaceRepositoryLocationNode[]; allRepos: DagsterRepoOption[]; visibleRepos: DagsterRepoOption[]; - data: RootWorkspaceQuery | null; + data: Record; + refetch: () => Promise; - refetch: () => Promise>; toggleVisible: SetVisibleOrHiddenFn; setVisible: SetVisibleOrHiddenFn; setHidden: SetVisibleOrHiddenFn; @@ -59,180 +68,216 @@ export const WorkspaceContext = React.createContext( export const HIDDEN_REPO_KEYS = 'dagster.hidden-repo-keys'; -export const ROOT_WORKSPACE_QUERY = gql` - query RootWorkspaceQuery { - workspaceOrError { - ... on Workspace { - id - locationEntries { - id - ...WorkspaceLocationNode - } +export const WorkspaceProvider = ({children}: {children: React.ReactNode}) => { + const {localCacheIdPrefix} = useContext(AppContext); + const codeLocationStatusQueryResult = useIndexedDBCachedQuery< + CodeLocationStatusQuery, + CodeLocationStatusQueryVariables + >({ + query: CODE_LOCATION_STATUS_QUERY, + version: CODE_LOCATION_STATUS_QUERY_VERSION, + key: `${localCacheIdPrefix}${CODE_LOCATION_STATUS_QUERY_KEY}`, + }); + if (typeof jest === 'undefined') { + // Only do this outside of jest for now so that we don't need to add RecoilRoot around everything... + // we will switch to jotai at some point instead... which doesnt require a + // eslint-disable-next-line react-hooks/rules-of-hooks + const setCodeLocationStatusAtom = useSetRecoilState(codeLocationStatusAtom); + // eslint-disable-next-line react-hooks/rules-of-hooks + useLayoutEffect(() => { + if (codeLocationStatusQueryResult.data) { + setCodeLocationStatusAtom(codeLocationStatusQueryResult.data); } - ...PythonErrorFragment - } + }, [codeLocationStatusQueryResult.data, setCodeLocationStatusAtom]); } + indexedDB.deleteDatabase('indexdbQueryCache:RootWorkspace'); - fragment WorkspaceLocationNode on WorkspaceLocationEntry { - id - name - loadStatus - displayMetadata { - ...WorkspaceDisplayMetadata - } - updatedTimestamp - featureFlags { - name - enabled - } - locationOrLoadError { - ... on RepositoryLocation { - id - ...WorkspaceLocation - } - ...PythonErrorFragment - } - } + const fetch = codeLocationStatusQueryResult.fetch; + useRefreshAtInterval({ + refresh: useCallback(async () => { + return await fetch(); + }, [fetch]), + intervalMs: 5000, + leading: true, + }); - fragment WorkspaceDisplayMetadata on RepositoryMetadata { - key - value - } + const {data, loading: loadingCodeLocationStatus} = codeLocationStatusQueryResult; - fragment WorkspaceLocation on RepositoryLocation { - id - isReloadSupported - serverId - name - dagsterLibraryVersions { - name - version - } - repositories { - id - ...WorkspaceRepository - } - } + const locations = useMemo(() => getLocations(data), [data]); + const prevLocations = useRef({}); - fragment WorkspaceRepository on Repository { - id - name - pipelines { - id - name - isJob - isAssetJob - pipelineSnapshotId - } - schedules { - id - ...WorkspaceSchedule - } - sensors { - id - ...WorkspaceSensor - } - partitionSets { - id - mode - pipelineName + const didInitiateFetchFromCache = useRef(false); + const [didLoadCachedData, setDidLoadCachedData] = useState(false); + + const [locationsData, setLocationsData] = React.useState< + Record + >({}); + + const getCachedData = useGetCachedData(); + const getData = useGetData(); + const clearCachedData = useClearCachedData(); + + useLayoutEffect(() => { + // Load data from the cache + if (didInitiateFetchFromCache.current) { + return; } - assetGroups { - id - groupName + didInitiateFetchFromCache.current = true; + new Promise(async (res) => { + /** + * 1. Load the cached code location status query + * 2. Load the cached data for those locations + * 3. Set the cached data to `locationsData` state + * 4. Set prevLocations equal to these cached locations so that we can check if they + * have changed after the next call to codeLocationStatusQuery + * 5. set didLoadCachedData to true to unblock the `locationsToFetch` memo so that it can compare + * the latest codeLocationStatusQuery result to what was in the cache. + */ + const data = await getCachedData({ + key: `${localCacheIdPrefix}${CODE_LOCATION_STATUS_QUERY_KEY}`, + version: CODE_LOCATION_STATUS_QUERY_VERSION, + }); + const cachedLocations = getLocations(data); + const prevCachedLocations: typeof locations = {}; + + await Promise.all([ + ...Object.values(cachedLocations).map(async (location) => { + const locationData = await getCachedData({ + key: `${localCacheIdPrefix}${locationWorkspaceKey(location.name)}`, + version: LOCATION_WORKSPACE_QUERY_VERSION, + }); + const entry = locationData?.workspaceLocationEntryOrError; + + if (!entry) { + return; + } + setLocationsData((locationsData) => + Object.assign({}, locationsData, { + [location.name]: entry, + }), + ); + + if (entry.__typename === 'WorkspaceLocationEntry') { + prevCachedLocations[location.name] = location; + } + }), + ]); + prevLocations.current = prevCachedLocations; + res(void 0); + }).then(() => { + setDidLoadCachedData(true); + }); + }, [getCachedData, localCacheIdPrefix, locations]); + + const client = useApolloClient(); + + const refetchLocation = useCallback( + async (name: string) => { + const locationData = await getData({ + client, + query: LOCATION_WORKSPACE_QUERY, + key: `${localCacheIdPrefix}${locationWorkspaceKey(name)}`, + version: LOCATION_WORKSPACE_QUERY_VERSION, + variables: { + name, + }, + bypassCache: true, + }); + const entry = locationData?.workspaceLocationEntryOrError; + setLocationsData((locationsData) => + Object.assign({}, locationsData, { + [name]: entry, + }), + ); + return locationData; + }, + [client, getData, localCacheIdPrefix], + ); + + const [isRefetching, setIsRefetching] = useState(false); + + const locationsToFetch = useMemo(() => { + if (!didLoadCachedData) { + return []; } - allTopLevelResourceDetails { - id - name + if (isRefetching) { + return []; } - ...RepositoryInfoFragment - } - - fragment WorkspaceSchedule on Schedule { - id - cronSchedule - executionTimezone - mode - name - pipelineName - scheduleState { - id - selectorId - status + const toFetch = Object.values(locations).filter((loc) => { + const prev = prevLocations.current?.[loc.name]; + const d = locationsData[loc.name]; + const entry = d?.__typename === 'WorkspaceLocationEntry' ? d : null; + return ( + prev?.updateTimestamp !== loc.updateTimestamp || + prev?.loadStatus !== loc.loadStatus || + entry?.loadStatus !== loc.loadStatus + ); + }); + prevLocations.current = locations; + return toFetch; + }, [didLoadCachedData, isRefetching, locations, locationsData]); + + useLayoutEffect(() => { + if (!locationsToFetch.length) { + return; } - } + setIsRefetching(true); + Promise.all( + locationsToFetch.map(async (location) => { + return await refetchLocation(location.name); + }), + ).then(() => { + setIsRefetching(false); + }); + }, [refetchLocation, locationsToFetch]); + + const locationsRemoved = useMemo( + () => + Array.from( + new Set([ + ...Object.values(prevLocations.current).filter((loc) => !locations[loc.name]), + ...Object.values(locationsData).filter( + (loc): loc is WorkspaceLocationNodeFragment => + loc.__typename === 'WorkspaceLocationEntry' && !locations[loc.name], + ), + ]), + ), + [locations, locationsData], + ); - fragment WorkspaceSensor on Sensor { - id - jobOriginId - name - targets { - mode - pipelineName + useLayoutEffect(() => { + if (!locationsRemoved.length) { + return; } - sensorState { - id - selectorId - status - ...BasicInstigationStateFragment + const copy = {...locationsData}; + locationsRemoved.forEach((loc) => { + delete copy[loc.name]; + clearCachedData({key: `${localCacheIdPrefix}${locationWorkspaceKey(loc.name)}`}); + }); + if (Object.keys(copy).length !== Object.keys(locationsData).length) { + setLocationsData(copy); } - sensorType - ...SensorSwitchFragment - } - - ${PYTHON_ERROR_FRAGMENT} - ${REPOSITORY_INFO_FRAGMENT} - ${SENSOR_SWITCH_FRAGMENT} - ${BASIC_INSTIGATION_STATE_FRAGMENT} -`; - -/** - * A hook that supplies the current workspace state of Dagster UI, including the current - * "active" repo based on the URL or localStorage, all fetched repositories available - * in the workspace, and loading/error state for the relevant query. - */ -const useWorkspaceState = (): WorkspaceState => { - const {localCacheIdPrefix} = useContext(AppContext); - const { - data, - loading, - fetch: refetch, - } = useIndexedDBCachedQuery({ - query: ROOT_WORKSPACE_QUERY, - key: `${localCacheIdPrefix}/RootWorkspace`, - version: 1, - }); - - // Delete old database from before the prefix, remove this at some point - indexedDB.deleteDatabase('indexdbQueryCache:RootWorkspace'); - useMemo(() => refetch(), [refetch]); - - const workspaceOrError = data?.workspaceOrError; - - const locationEntries = React.useMemo( - () => (workspaceOrError?.__typename === 'Workspace' ? workspaceOrError.locationEntries : []), - [workspaceOrError], + }, [clearCachedData, localCacheIdPrefix, locationsData, locationsRemoved]); + + const locationEntries = useMemo( + () => + Object.values(locationsData).filter( + (entry): entry is WorkspaceLocationNodeFragment => + entry.__typename === 'WorkspaceLocationEntry', + ), + [locationsData], ); - const {allRepos, error} = React.useMemo(() => { + const allRepos = React.useMemo(() => { let allRepos: DagsterRepoOption[] = []; - if (!workspaceOrError) { - return {allRepos, error: null}; - } - - if (workspaceOrError.__typename === 'PythonError') { - return {allRepos, error: workspaceOrError}; - } allRepos = sortBy( - workspaceOrError.locationEntries.reduce((accum, locationEntry) => { + locationEntries.reduce((accum, locationEntry) => { if (locationEntry.locationOrLoadError?.__typename !== 'RepositoryLocation') { return accum; } const repositoryLocation = locationEntry.locationOrLoadError; - const reposForLocation = repositoryLocation.repositories.map((repository) => { - return {repository, repositoryLocation}; - }); + const reposForLocation = repoLocationToRepos(repositoryLocation); return [...accum, ...reposForLocation]; }, [] as DagsterRepoOption[]), @@ -240,25 +285,60 @@ const useWorkspaceState = (): WorkspaceState => { (r) => `${r.repositoryLocation.name}:${r.repository.name}`, ); - return {error: null, allRepos}; - }, [workspaceOrError]); + return allRepos; + }, [locationEntries]); const {visibleRepos, toggleVisible, setVisible, setHidden} = useVisibleRepos(allRepos); - return { - refetch, - loading: loading && !data, // Only "loading" on initial load. - error, - locationEntries, - data, - allRepos, - visibleRepos, - toggleVisible, - setVisible, - setHidden, - }; + const locationsRef = useUpdatingRef(locations); + + const refetch = useCallback(async () => { + return await Promise.all( + Object.values(locationsRef.current).map((location) => refetchLocation(location.name)), + ); + }, [locationsRef, refetchLocation]); + + return ( + + {children} + + ); }; +function getLocations(d: CodeLocationStatusQuery | undefined | null) { + const locations = + d?.locationStatusesOrError?.__typename === 'WorkspaceLocationStatusEntries' + ? d?.locationStatusesOrError.entries + : []; + + return locations.reduce( + (accum, loc) => { + accum[loc.name] = loc; + return accum; + }, + {} as Record, + ); +} + +export function locationWorkspaceKey(name: string) { + return `/LocationWorkspace/${name}`; +} + /** * useVisibleRepos returns `{reposForKeys, toggleVisible, setVisible, setHidden}` and internally * mirrors the current selection into localStorage so that the default selection in new browser @@ -355,16 +435,9 @@ const useVisibleRepos = ( const getRepositoryOptionHash = (a: DagsterRepoOption) => `${a.repository.name}:${a.repositoryLocation.name}`; - -export const WorkspaceProvider = ({children}: {children: React.ReactNode}) => { - const workspaceState = useWorkspaceState(); - - return {children}; -}; - export const useRepositoryOptions = () => { - const {allRepos: options, loading, error} = React.useContext(WorkspaceContext); - return {options, loading, error}; + const {allRepos: options, loading} = React.useContext(WorkspaceContext); + return {options, loading}; }; export const useRepository = (repoAddress: RepoAddress | null) => { @@ -412,7 +485,7 @@ export const getFeatureFlagForCodeLocation = ( }; export const useFeatureFlagForCodeLocation = (locationName: string, flagName: string) => { - const {locationEntries} = useWorkspaceState(); + const {locationEntries} = useContext(WorkspaceContext); return getFeatureFlagForCodeLocation(locationEntries, locationName, flagName); }; @@ -452,3 +525,9 @@ export const buildPipelineSelector = ( export const optionToRepoAddress = (option: DagsterRepoOption) => buildRepoAddress(option.repository.name, option.repository.location.name); + +export function repoLocationToRepos(repositoryLocation: RepositoryLocation) { + return repositoryLocation.repositories.map((repository) => { + return {repository, repositoryLocation}; + }); +} diff --git a/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceQueries.tsx b/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceQueries.tsx new file mode 100644 index 0000000000000..cc253de00c2b2 --- /dev/null +++ b/js_modules/dagster-ui/packages/ui-core/src/workspace/WorkspaceQueries.tsx @@ -0,0 +1,140 @@ +import {gql} from '@apollo/client'; + +import {REPOSITORY_INFO_FRAGMENT} from './RepositoryInformation'; +import {PYTHON_ERROR_FRAGMENT} from '../app/PythonErrorFragment'; +import {BASIC_INSTIGATION_STATE_FRAGMENT} from '../overview/BasicInstigationStateFragment'; +import {SENSOR_SWITCH_FRAGMENT} from '../sensors/SensorSwitch'; + +export const LOCATION_WORKSPACE_QUERY = gql` + query LocationWorkspaceQuery($name: String!) { + workspaceLocationEntryOrError(name: $name) { + ...WorkspaceLocationNode + } + } + + fragment WorkspaceLocationNode on WorkspaceLocationEntry { + id + name + loadStatus + displayMetadata { + ...WorkspaceDisplayMetadata + } + updatedTimestamp + featureFlags { + name + enabled + } + locationOrLoadError { + ... on RepositoryLocation { + id + ...WorkspaceLocation + } + ...PythonErrorFragment + } + } + + fragment WorkspaceDisplayMetadata on RepositoryMetadata { + key + value + } + + fragment WorkspaceLocation on RepositoryLocation { + id + isReloadSupported + serverId + name + dagsterLibraryVersions { + name + version + } + repositories { + id + ...WorkspaceRepository + } + } + + fragment WorkspaceRepository on Repository { + id + name + pipelines { + id + name + isJob + isAssetJob + pipelineSnapshotId + } + schedules { + id + ...WorkspaceSchedule + } + sensors { + id + ...WorkspaceSensor + } + partitionSets { + id + mode + pipelineName + } + assetGroups { + id + groupName + } + allTopLevelResourceDetails { + id + name + } + ...RepositoryInfoFragment + } + + fragment WorkspaceSchedule on Schedule { + id + cronSchedule + executionTimezone + mode + name + pipelineName + scheduleState { + id + selectorId + status + } + } + + fragment WorkspaceSensor on Sensor { + id + jobOriginId + name + targets { + mode + pipelineName + } + sensorState { + id + selectorId + status + ...BasicInstigationStateFragment + } + sensorType + ...SensorSwitchFragment + } + ${BASIC_INSTIGATION_STATE_FRAGMENT} + ${SENSOR_SWITCH_FRAGMENT} + ${PYTHON_ERROR_FRAGMENT} + ${REPOSITORY_INFO_FRAGMENT} +`; + +export const CODE_LOCATION_STATUS_QUERY = gql` + query CodeLocationStatusQuery { + locationStatusesOrError { + ... on WorkspaceLocationStatusEntries { + entries { + id + name + loadStatus + updateTimestamp + } + } + } + } +`; diff --git a/js_modules/dagster-ui/packages/ui-core/src/workspace/__fixtures__/Workspace.fixtures.ts b/js_modules/dagster-ui/packages/ui-core/src/workspace/__fixtures__/Workspace.fixtures.ts new file mode 100644 index 0000000000000..a90580d1959a7 --- /dev/null +++ b/js_modules/dagster-ui/packages/ui-core/src/workspace/__fixtures__/Workspace.fixtures.ts @@ -0,0 +1,62 @@ +import {MockedResponse} from '@apollo/client/testing'; + +import { + WorkspaceLocationEntry, + WorkspaceLocationStatusEntry, + buildWorkspaceLocationStatusEntries, + buildWorkspaceLocationStatusEntry, +} from '../../graphql/types'; +import {buildQueryMock} from '../../testing/mocking'; +import {CODE_LOCATION_STATUS_QUERY, LOCATION_WORKSPACE_QUERY} from '../WorkspaceQueries'; +import { + CodeLocationStatusQuery, + CodeLocationStatusQueryVariables, + LocationWorkspaceQuery, + LocationWorkspaceQueryVariables, +} from '../types/WorkspaceQueries.types'; + +export const buildCodeLocationsStatusQuery = ( + entries: WorkspaceLocationStatusEntry[], + options: Partial> = {}, +): MockedResponse => { + return buildQueryMock({ + query: CODE_LOCATION_STATUS_QUERY, + variables: {}, + data: { + locationStatusesOrError: buildWorkspaceLocationStatusEntries({ + entries, + }), + }, + ...options, + }); +}; + +export const buildWorkspaceMocks = ( + entries: WorkspaceLocationEntry[], + options: Partial> = {}, +) => { + return [ + buildCodeLocationsStatusQuery( + entries.map((entry) => + buildWorkspaceLocationStatusEntry({ + ...entry, + updateTimestamp: entry.updatedTimestamp, + __typename: 'WorkspaceLocationStatusEntry', + }), + ), + options, + ), + ...entries.map((entry) => + buildQueryMock({ + query: LOCATION_WORKSPACE_QUERY, + variables: { + name: entry.name, + }, + data: { + workspaceLocationEntryOrError: entry, + }, + ...options, + }), + ), + ]; +}; diff --git a/js_modules/dagster-ui/packages/ui-core/src/workspace/__tests__/WorkspaceContext.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/workspace/__tests__/WorkspaceContext.test.tsx new file mode 100644 index 0000000000000..e0d47c18667ac --- /dev/null +++ b/js_modules/dagster-ui/packages/ui-core/src/workspace/__tests__/WorkspaceContext.test.tsx @@ -0,0 +1,436 @@ +jest.useFakeTimers(); + +import {MockedProvider, MockedResponse} from '@apollo/client/testing'; +import {act, renderHook, waitFor} from '@testing-library/react'; +import {cache} from 'idb-lru-cache'; +import {useContext} from 'react'; +import {RecoilRoot} from 'recoil'; + +import {AppContext} from '../../app/AppContext'; +import { + buildRepository, + buildRepositoryLocation, + buildWorkspaceLocationEntry, +} from '../../graphql/types'; +import { + IndexedDBCacheContext, + KEY_PREFIX, + createIndexedDBCacheContextValue, +} from '../../search/useIndexedDBCachedQuery'; +import {getMockResultFn} from '../../testing/mocking'; +import { + CODE_LOCATION_STATUS_QUERY_KEY, + CODE_LOCATION_STATUS_QUERY_VERSION, + LOCATION_WORKSPACE_QUERY_VERSION, + WorkspaceContext, + WorkspaceProvider, + locationWorkspaceKey, + repoLocationToRepos, +} from '../WorkspaceContext'; +import {buildWorkspaceMocks} from '../__fixtures__/Workspace.fixtures'; + +const mockCache = cache as any; + +const mockedCacheStore: Record = {}; + +jest.mock('idb-lru-cache', () => { + return { + cache: jest.fn(({dbName}) => { + if (!mockedCacheStore[dbName]) { + mockedCacheStore[dbName] = { + has: jest.fn(), + get: jest.fn(), + set: jest.fn(), + delete: jest.fn(), + }; + } + return mockedCacheStore[dbName]; + }), + }; +}); + +afterEach(async () => { + jest.resetModules(); + jest.clearAllMocks(); + jest.clearAllTimers(); +}); +const LOCAL_CACHE_ID_PREFIX = 'test'; +function renderWithMocks(mocks: MockedResponse[]) { + return renderHook(() => useContext(WorkspaceContext), { + wrapper({children}) { + return ( + + + + + {children} + + + + + ); + }, + }); +} + +function getLocationMocks(updatedTimestamp = 0) { + const repositoryLocation1 = buildRepositoryLocation({ + repositories: [buildRepository(), buildRepository()], + }); + const repositoryLocation2 = buildRepositoryLocation({ + repositories: [buildRepository(), buildRepository()], + }); + const repositoryLocation3 = buildRepositoryLocation({ + repositories: [buildRepository(), buildRepository()], + }); + const location1 = buildWorkspaceLocationEntry({ + name: 'location1', + updatedTimestamp, + locationOrLoadError: repositoryLocation1, + }); + const location2 = buildWorkspaceLocationEntry({ + name: 'location2', + updatedTimestamp, + locationOrLoadError: repositoryLocation2, + }); + + const location3 = buildWorkspaceLocationEntry({ + name: 'location3', + updatedTimestamp, + locationOrLoadError: repositoryLocation3, + }); + + return { + repositoryLocation1, + repositoryLocation2, + repositoryLocation3, + location1, + location2, + location3, + caches: { + codeLocationStatusQuery: mockCache({ + dbName: `${KEY_PREFIX}${LOCAL_CACHE_ID_PREFIX}${CODE_LOCATION_STATUS_QUERY_KEY}`, + }), + location1: mockCache({ + dbName: `${KEY_PREFIX}${LOCAL_CACHE_ID_PREFIX}${locationWorkspaceKey('location1')}`, + }), + location2: mockCache({ + dbName: `${KEY_PREFIX}${LOCAL_CACHE_ID_PREFIX}${locationWorkspaceKey('location2')}`, + }), + location3: mockCache({ + dbName: `${KEY_PREFIX}${LOCAL_CACHE_ID_PREFIX}${locationWorkspaceKey('location3')}`, + }), + }, + }; +} + +describe('WorkspaceContext', () => { + it('Fetches by code location when cache is empty', async () => { + const { + location1, + location2, + location3, + repositoryLocation1, + repositoryLocation2, + repositoryLocation3, + caches, + } = getLocationMocks(-1); + caches.codeLocationStatusQuery.has.mockResolvedValue(false); + caches.location1.has.mockResolvedValue(false); + caches.location2.has.mockResolvedValue(false); + caches.location3.has.mockResolvedValue(false); + const mocks = buildWorkspaceMocks([location1, location2, location3], {delay: 10}); + const mockCbs = mocks.map(getMockResultFn); + + // Include code location status mock a second time since we call runOnlyPendingTimersAsync twice + const {result} = renderWithMocks([...mocks, mocks[0]!]); + + expect(result.current.allRepos).toEqual([]); + expect(result.current.data).toEqual({}); + expect(result.current.loading).toEqual(true); + + // Runs the code location status query + await act(async () => { + await jest.runOnlyPendingTimersAsync(); + }); + + // First mock is the code location status query + expect(mockCbs[0]).toHaveBeenCalled(); + expect(mockCbs[1]).not.toHaveBeenCalled(); + expect(mockCbs[2]).not.toHaveBeenCalled(); + expect(mockCbs[3]).not.toHaveBeenCalled(); + + expect(result.current.allRepos).toEqual([]); + expect(result.current.data).toEqual({}); + expect(result.current.loading).toEqual(true); + + // Runs the individual location queries + await act(async () => { + await jest.runOnlyPendingTimersAsync(); + }); + + expect(mockCbs[1]).toHaveBeenCalled(); + expect(mockCbs[2]).toHaveBeenCalled(); + expect(mockCbs[3]).toHaveBeenCalled(); + + await waitFor(() => { + expect(result.current.loading).toEqual(false); + }); + expect(result.current.allRepos).toEqual([ + ...repoLocationToRepos(repositoryLocation1), + ...repoLocationToRepos(repositoryLocation2), + ...repoLocationToRepos(repositoryLocation3), + ]); + expect(result.current.data).toEqual({ + [location1.name]: location1, + [location2.name]: location2, + [location3.name]: location3, + }); + + await act(async () => { + // Exhaust any remaining tasks so they don't affect the next test. + await jest.runAllTicks(); + }); + }); + + it('Uses cache if it is up to date and does not query by location', async () => { + const { + location1, + location2, + location3, + repositoryLocation1, + repositoryLocation2, + repositoryLocation3, + caches, + } = getLocationMocks(); + const mocks = buildWorkspaceMocks([location1, location2, location3], {delay: 10}); + + caches.codeLocationStatusQuery.has.mockResolvedValue(true); + caches.codeLocationStatusQuery.get.mockResolvedValue({ + value: {data: (mocks[0]! as any).result.data, version: CODE_LOCATION_STATUS_QUERY_VERSION}, + }); + caches.location1.has.mockResolvedValue(true); + caches.location1.get.mockResolvedValue({ + value: {data: (mocks[1]! as any).result.data, version: LOCATION_WORKSPACE_QUERY_VERSION}, + }); + caches.location2.has.mockResolvedValue(true); + caches.location2.get.mockResolvedValue({ + value: {data: (mocks[2]! as any).result.data, version: LOCATION_WORKSPACE_QUERY_VERSION}, + }); + caches.location3.has.mockResolvedValue(true); + caches.location3.get.mockResolvedValue({ + value: {data: (mocks[3]! as any).result.data, version: LOCATION_WORKSPACE_QUERY_VERSION}, + }); + + const mockCbs = mocks.map(getMockResultFn); + + const {result} = renderWithMocks([...mocks]); + + // await act(async () => { + // await jest.runOnlyPendingTimersAsync(); + // }); + + await waitFor(async () => { + expect(result.current.loading).toEqual(false); + }); + // We queries for code location statuses but saw we were up to date + // so we didn't call any the location queries + expect(mockCbs[0]).toHaveBeenCalled(); + + expect(mockCbs[1]).not.toHaveBeenCalled(); + expect(mockCbs[2]).not.toHaveBeenCalled(); + expect(mockCbs[3]).not.toHaveBeenCalled(); + expect(result.current.allRepos).toEqual([ + ...repoLocationToRepos(repositoryLocation1), + ...repoLocationToRepos(repositoryLocation2), + ...repoLocationToRepos(repositoryLocation3), + ]); + expect(result.current.data).toEqual({ + [location1.name]: location1, + [location2.name]: location2, + [location3.name]: location3, + }); + + await act(async () => { + // Exhaust any remaining tasks so they don't affect the next test. + await jest.runAllTicks(); + }); + }); + + it('returns cached data, detects its out of date and queries by location to update it', async () => { + const { + location1, + location2, + location3, + repositoryLocation1, + repositoryLocation2, + repositoryLocation3, + caches, + } = getLocationMocks(-3); + const mocks = buildWorkspaceMocks([location1, location2, location3], {delay: 10}); + const { + location1: updatedLocation1, + location2: updatedLocation2, + location3: updatedLocation3, + } = getLocationMocks(1); + + const updatedMocks = buildWorkspaceMocks( + [updatedLocation1, updatedLocation2, updatedLocation3], + {delay: 10}, + ); + + caches.codeLocationStatusQuery.has.mockResolvedValue(true); + caches.codeLocationStatusQuery.get.mockResolvedValue({ + value: {data: (mocks[0]! as any).result.data, version: CODE_LOCATION_STATUS_QUERY_VERSION}, + }); + caches.location1.has.mockResolvedValue(true); + caches.location1.get.mockResolvedValue({ + value: {data: (mocks[1]! as any).result.data, version: LOCATION_WORKSPACE_QUERY_VERSION}, + }); + caches.location2.has.mockResolvedValue(true); + caches.location2.get.mockResolvedValue({ + value: {data: (mocks[2]! as any).result.data, version: LOCATION_WORKSPACE_QUERY_VERSION}, + }); + caches.location3.has.mockResolvedValue(true); + caches.location3.get.mockResolvedValue({ + value: {data: (mocks[3]! as any).result.data, version: LOCATION_WORKSPACE_QUERY_VERSION}, + }); + const mockCbs = updatedMocks.map(getMockResultFn); + + const {result} = renderWithMocks([...updatedMocks, updatedMocks[0]!]); + + await act(async () => { + await jest.runOnlyPendingTimersAsync(); + }); + // We queries for code location statuses and we see we are not up to date yet so the current data is still equal to the cached data + expect(mockCbs[0]).toHaveBeenCalled(); + expect(mockCbs[1]).not.toHaveBeenCalled(); + expect(mockCbs[2]).not.toHaveBeenCalled(); + expect(mockCbs[3]).not.toHaveBeenCalled(); + + expect(result.current.allRepos).toEqual([ + ...repoLocationToRepos(repositoryLocation1), + ...repoLocationToRepos(repositoryLocation2), + ...repoLocationToRepos(repositoryLocation3), + ]); + expect(result.current.data).toEqual({ + [location1.name]: location1, + [location2.name]: location2, + [location3.name]: location3, + }); + + // Run the location queries and wait for the location queries to return + await act(async () => { + await jest.runOnlyPendingTimersAsync(); + }); + + expect(result.current.data).toEqual({ + [location1.name]: updatedLocation1, + [location2.name]: updatedLocation2, + [location3.name]: updatedLocation3, + }); + expect(updatedLocation1).not.toEqual(location1); + + await act(async () => { + // Exhaust any remaining tasks so they don't affect the next test. + await jest.runAllTicks(); + }); + }); + + it('Detects and deletes removed code locations from cache', async () => { + const { + location1, + location2, + location3, + repositoryLocation1, + repositoryLocation2, + repositoryLocation3, + caches, + } = getLocationMocks(0); + const mocks = buildWorkspaceMocks([location1, location2, location3], {delay: 10}); + const {location1: updatedLocation1, location2: updatedLocation2} = getLocationMocks(1); + + // Remove location 3 + const updatedMocks = buildWorkspaceMocks([updatedLocation1, updatedLocation2], {delay: 10}); + + caches.codeLocationStatusQuery.has.mockResolvedValue(true); + caches.codeLocationStatusQuery.get.mockResolvedValue({ + value: {data: (mocks[0]! as any).result.data, version: CODE_LOCATION_STATUS_QUERY_VERSION}, + }); + caches.location1.has.mockResolvedValue(true); + caches.location1.get.mockResolvedValue({ + value: {data: (mocks[1]! as any).result.data, version: LOCATION_WORKSPACE_QUERY_VERSION}, + }); + caches.location2.has.mockResolvedValue(true); + caches.location2.get.mockResolvedValue({ + value: {data: (mocks[2]! as any).result.data, version: LOCATION_WORKSPACE_QUERY_VERSION}, + }); + caches.location3.has.mockResolvedValue(true); + caches.location3.get.mockResolvedValue({ + value: {data: (mocks[3]! as any).result.data, version: LOCATION_WORKSPACE_QUERY_VERSION}, + }); + const mockCbs = updatedMocks.map(getMockResultFn); + + const {result} = renderWithMocks([...updatedMocks, updatedMocks[0]!]); + + await act(async () => { + await jest.runAllTicks(); + }); + + await waitFor(async () => { + expect(result.current.loading).toEqual(false); + }); + + // We return the cached data + expect(result.current.allRepos).toEqual([ + ...repoLocationToRepos(repositoryLocation1), + ...repoLocationToRepos(repositoryLocation2), + ...repoLocationToRepos(repositoryLocation3), + ]); + + expect(result.current.data).toEqual({ + [location1.name]: location1, + [location2.name]: location2, + [location3.name]: location3, + }); + + expect(mockCbs[0]).toHaveBeenCalled(); + expect(mockCbs[1]).not.toHaveBeenCalled(); + expect(mockCbs[2]).not.toHaveBeenCalled(); + + await act(async () => { + // Our mocks have a delay of 10 + jest.advanceTimersByTime(10); + jest.runAllTicks(); + }); + + // We detect the third code location was deleted + expect(result.current.allRepos).toEqual([ + ...repoLocationToRepos(repositoryLocation1), + ...repoLocationToRepos(repositoryLocation2), + ]); + + expect(result.current.data).toEqual({ + [location1.name]: location1, + [location2.name]: location2, + }); + + // We query for the the updated cached locations + await act(async () => { + await jest.runOnlyPendingTimersAsync(); + }); + + // We now have the latest data + expect(result.current.data).toEqual({ + [location1.name]: updatedLocation1, + [location2.name]: updatedLocation2, + }); + }); +}); diff --git a/js_modules/dagster-ui/packages/ui-core/src/workspace/__tests__/getFeatureFlagForCodeLocation.test.tsx b/js_modules/dagster-ui/packages/ui-core/src/workspace/__tests__/getFeatureFlagForCodeLocation.test.tsx index 6c38a4ce3b699..0bfb1480559df 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/workspace/__tests__/getFeatureFlagForCodeLocation.test.tsx +++ b/js_modules/dagster-ui/packages/ui-core/src/workspace/__tests__/getFeatureFlagForCodeLocation.test.tsx @@ -1,6 +1,6 @@ import {buildFeatureFlag, buildWorkspaceLocationEntry} from '../../graphql/types'; import {getFeatureFlagForCodeLocation} from '../WorkspaceContext'; -import {WorkspaceLocationNodeFragment} from '../types/WorkspaceContext.types'; +import {WorkspaceLocationNodeFragment} from '../types/WorkspaceQueries.types'; describe('getFeatureFlagForCodeLocation', () => { const locationEntries: WorkspaceLocationNodeFragment[] = [ diff --git a/js_modules/dagster-ui/packages/ui-core/src/workspace/types/WorkspaceContext.types.ts b/js_modules/dagster-ui/packages/ui-core/src/workspace/types/WorkspaceQueries.types.ts similarity index 66% rename from js_modules/dagster-ui/packages/ui-core/src/workspace/types/WorkspaceContext.types.ts rename to js_modules/dagster-ui/packages/ui-core/src/workspace/types/WorkspaceQueries.types.ts index 79e87d591d4c2..720879ed0d097 100644 --- a/js_modules/dagster-ui/packages/ui-core/src/workspace/types/WorkspaceContext.types.ts +++ b/js_modules/dagster-ui/packages/ui-core/src/workspace/types/WorkspaceQueries.types.ts @@ -2,128 +2,114 @@ import * as Types from '../../graphql/types'; -export type RootWorkspaceQueryVariables = Types.Exact<{[key: string]: never}>; +export type LocationWorkspaceQueryVariables = Types.Exact<{ + name: Types.Scalars['String']['input']; +}>; -export type RootWorkspaceQuery = { +export type LocationWorkspaceQuery = { __typename: 'Query'; - workspaceOrError: + workspaceLocationEntryOrError: + | {__typename: 'PythonError'} | { - __typename: 'PythonError'; - message: string; - stack: Array; - errorChain: Array<{ - __typename: 'ErrorChainLink'; - isExplicitLink: boolean; - error: {__typename: 'PythonError'; message: string; stack: Array}; - }>; - } - | { - __typename: 'Workspace'; + __typename: 'WorkspaceLocationEntry'; id: string; - locationEntries: Array<{ - __typename: 'WorkspaceLocationEntry'; - id: string; - name: string; - loadStatus: Types.RepositoryLocationLoadStatus; - updatedTimestamp: number; - displayMetadata: Array<{__typename: 'RepositoryMetadata'; key: string; value: string}>; - featureFlags: Array<{__typename: 'FeatureFlag'; name: string; enabled: boolean}>; - locationOrLoadError: - | { - __typename: 'PythonError'; - message: string; - stack: Array; - errorChain: Array<{ - __typename: 'ErrorChainLink'; - isExplicitLink: boolean; - error: {__typename: 'PythonError'; message: string; stack: Array}; - }>; - } - | { - __typename: 'RepositoryLocation'; + name: string; + loadStatus: Types.RepositoryLocationLoadStatus; + updatedTimestamp: number; + displayMetadata: Array<{__typename: 'RepositoryMetadata'; key: string; value: string}>; + featureFlags: Array<{__typename: 'FeatureFlag'; name: string; enabled: boolean}>; + locationOrLoadError: + | { + __typename: 'PythonError'; + message: string; + stack: Array; + errorChain: Array<{ + __typename: 'ErrorChainLink'; + isExplicitLink: boolean; + error: {__typename: 'PythonError'; message: string; stack: Array}; + }>; + } + | { + __typename: 'RepositoryLocation'; + id: string; + isReloadSupported: boolean; + serverId: string | null; + name: string; + dagsterLibraryVersions: Array<{ + __typename: 'DagsterLibraryVersion'; + name: string; + version: string; + }> | null; + repositories: Array<{ + __typename: 'Repository'; id: string; - isReloadSupported: boolean; - serverId: string | null; name: string; - dagsterLibraryVersions: Array<{ - __typename: 'DagsterLibraryVersion'; + pipelines: Array<{ + __typename: 'Pipeline'; + id: string; name: string; - version: string; - }> | null; - repositories: Array<{ - __typename: 'Repository'; + isJob: boolean; + isAssetJob: boolean; + pipelineSnapshotId: string; + }>; + schedules: Array<{ + __typename: 'Schedule'; id: string; + cronSchedule: string; + executionTimezone: string | null; + mode: string; name: string; - pipelines: Array<{ - __typename: 'Pipeline'; - id: string; - name: string; - isJob: boolean; - isAssetJob: boolean; - pipelineSnapshotId: string; - }>; - schedules: Array<{ - __typename: 'Schedule'; + pipelineName: string; + scheduleState: { + __typename: 'InstigationState'; id: string; - cronSchedule: string; - executionTimezone: string | null; - mode: string; - name: string; - pipelineName: string; - scheduleState: { - __typename: 'InstigationState'; - id: string; - selectorId: string; - status: Types.InstigationStatus; - }; - }>; - sensors: Array<{ - __typename: 'Sensor'; - id: string; - jobOriginId: string; - name: string; - sensorType: Types.SensorType; - targets: Array<{ - __typename: 'Target'; - mode: string; - pipelineName: string; - }> | null; - sensorState: { - __typename: 'InstigationState'; - id: string; - selectorId: string; - status: Types.InstigationStatus; - hasStartPermission: boolean; - hasStopPermission: boolean; - typeSpecificData: - | {__typename: 'ScheduleData'} - | {__typename: 'SensorData'; lastCursor: string | null} - | null; - }; - }>; - partitionSets: Array<{ - __typename: 'PartitionSet'; - id: string; - mode: string; - pipelineName: string; - }>; - assetGroups: Array<{__typename: 'AssetGroup'; id: string; groupName: string}>; - allTopLevelResourceDetails: Array<{ - __typename: 'ResourceDetails'; + selectorId: string; + status: Types.InstigationStatus; + }; + }>; + sensors: Array<{ + __typename: 'Sensor'; + id: string; + jobOriginId: string; + name: string; + sensorType: Types.SensorType; + targets: Array<{__typename: 'Target'; mode: string; pipelineName: string}> | null; + sensorState: { + __typename: 'InstigationState'; id: string; - name: string; - }>; - location: {__typename: 'RepositoryLocation'; id: string; name: string}; - displayMetadata: Array<{ - __typename: 'RepositoryMetadata'; - key: string; - value: string; - }>; + selectorId: string; + status: Types.InstigationStatus; + hasStartPermission: boolean; + hasStopPermission: boolean; + typeSpecificData: + | {__typename: 'ScheduleData'} + | {__typename: 'SensorData'; lastCursor: string | null} + | null; + }; }>; - } - | null; - }>; - }; + partitionSets: Array<{ + __typename: 'PartitionSet'; + id: string; + mode: string; + pipelineName: string; + }>; + assetGroups: Array<{__typename: 'AssetGroup'; id: string; groupName: string}>; + allTopLevelResourceDetails: Array<{ + __typename: 'ResourceDetails'; + id: string; + name: string; + }>; + location: {__typename: 'RepositoryLocation'; id: string; name: string}; + displayMetadata: Array<{ + __typename: 'RepositoryMetadata'; + key: string; + value: string; + }>; + }>; + } + | null; + } + | null; }; export type WorkspaceLocationNodeFragment = { @@ -394,3 +380,21 @@ export type WorkspaceSensorFragment = { | null; }; }; + +export type CodeLocationStatusQueryVariables = Types.Exact<{[key: string]: never}>; + +export type CodeLocationStatusQuery = { + __typename: 'Query'; + locationStatusesOrError: + | {__typename: 'PythonError'} + | { + __typename: 'WorkspaceLocationStatusEntries'; + entries: Array<{ + __typename: 'WorkspaceLocationStatusEntry'; + id: string; + name: string; + loadStatus: Types.RepositoryLocationLoadStatus; + updateTimestamp: number; + }>; + }; +}; diff --git a/js_modules/dagster-ui/yarn.lock b/js_modules/dagster-ui/yarn.lock index b0e76f252ee33..67f55c477689c 100644 --- a/js_modules/dagster-ui/yarn.lock +++ b/js_modules/dagster-ui/yarn.lock @@ -2486,7 +2486,7 @@ __metadata: "@storybook/react-webpack5": "npm:^7.2.0" "@testing-library/dom": "npm:^10.0.0" "@testing-library/jest-dom": "npm:^6.4.2" - "@testing-library/react": "npm:^15.0.3" + "@testing-library/react": "npm:^16.0.0" "@testing-library/user-event": "npm:^14.5.2" "@types/babel__core": "npm:^7" "@types/babel__preset-env": "npm:^7" @@ -2576,7 +2576,7 @@ __metadata: "@tanstack/react-virtual": "npm:^3.0.1" "@testing-library/dom": "npm:^10.0.0" "@testing-library/jest-dom": "npm:^6.4.2" - "@testing-library/react": "npm:^15.0.3" + "@testing-library/react": "npm:^16.0.0" "@testing-library/react-hooks": "npm:^7.0.2" "@testing-library/user-event": "npm:^14.5.2" "@types/codemirror": "npm:^5.60.5" @@ -7335,17 +7335,23 @@ __metadata: languageName: node linkType: hard -"@testing-library/react@npm:^15.0.3": - version: 15.0.3 - resolution: "@testing-library/react@npm:15.0.3" +"@testing-library/react@npm:^16.0.0": + version: 16.0.0 + resolution: "@testing-library/react@npm:16.0.0" dependencies: "@babel/runtime": "npm:^7.12.5" - "@testing-library/dom": "npm:^10.0.0" - "@types/react-dom": "npm:^18.0.0" peerDependencies: + "@testing-library/dom": ^10.0.0 + "@types/react": ^18.0.0 + "@types/react-dom": ^18.0.0 react: ^18.0.0 react-dom: ^18.0.0 - checksum: 10/3e094accf49bdfba141ac53cad4902831fbef9d2d4a2c39a908820b141569aa8ca85e35f95560749ab46ad1db1be38a5382d38a0cb41a55a5d0b8afe116f2ba0 + peerDependenciesMeta: + "@types/react": + optional: true + "@types/react-dom": + optional: true + checksum: 10/b32894be94e31276138decfa6bcea69dfebc0c37cf91499ff6c878f41eb1154a43a7df6eb1e72e7bede78468af6cb67ca59e4acd3206b41f3ecdae2c6efdf67e languageName: node linkType: hard @@ -8006,7 +8012,7 @@ __metadata: languageName: node linkType: hard -"@types/react-dom@npm:>=16.9.0, @types/react-dom@npm:^18.0.0, @types/react-dom@npm:^18.2.0": +"@types/react-dom@npm:>=16.9.0, @types/react-dom@npm:^18.2.0": version: 18.2.7 resolution: "@types/react-dom@npm:18.2.7" dependencies: