Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UI] Fix metadata tabs loading state #2508

Merged
merged 3 commits into from
Oct 29, 2019
Merged

[UI] Fix metadata tabs loading state #2508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
39cc162
Fix spinner for ArtifactList page
Bobgy Oct 29, 2019
3866d15
Fix ExecutionList loading state
Bobgy Oct 29, 2019
3a36ad1
Helper to add delay to mock api server
Bobgy Oct 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions frontend/mock-backend/mock-api-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ import mockApiMiddleware from './mock-api-middleware';
const app = express();
const port = process.argv[2] || 3001;

// Uncomment the following line to get 1000ms delay to all requests
// app.use((req, res, next) => { setTimeout(next, 1000); });

app.use((_, res, next) => {
res.header('Access-Control-Allow-Origin', '*');
res.header('Access-Control-Allow-Headers', 'X-Requested-With, content-type');
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/lib/Apis.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ const metadataServicePromiseClient = {
getArtifactTypes: makePromiseApi(
metadataServiceClient.getArtifactTypes.bind(metadataServiceClient),
),
getArtifacts: makePromiseApi(metadataServiceClient.getArtifacts.bind(metadataServiceClient)),
getArtifactsByID: makePromiseApi(
metadataServiceClient.getArtifactsByID.bind(metadataServiceClient),
),
Expand All @@ -111,6 +112,10 @@ const metadataServicePromiseClient = {
getEventsByExecutionIDs: makePromiseApi(
metadataServiceClient.getEventsByExecutionIDs.bind(metadataServiceClient),
),
getExecutionTypes: makePromiseApi(
metadataServiceClient.getExecutionTypes.bind(metadataServiceClient),
),
getExecutions: makePromiseApi(metadataServiceClient.getExecutions.bind(metadataServiceClient)),
getExecutionsByID: makePromiseApi(
metadataServiceClient.getExecutionsByID.bind(metadataServiceClient),
),
Expand Down
144 changes: 74 additions & 70 deletions frontend/src/pages/ArtifactList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,18 +120,21 @@ class ArtifactList extends Page<{}, ArtifactListState> {
}

private async reload(request: ListRequest): Promise<string> {
Apis.getMetadataServiceClient().getArtifacts(new GetArtifactsRequest(), (err, res) => {
const { response: res, error: err } = await Apis.getMetadataServicePromiseClient().getArtifacts(
new GetArtifactsRequest(),
);

if (err) {
// Code === 5 means no record found in backend. This is a temporary workaround.
// TODO: remove err.code !== 5 check when backend is fixed.
if (err && err.code !== 5) {
if (err.code !== 5) {
this.showPageError(serviceErrorToString(err));
return;
}
return '';
}

const artifacts = (res && res.getArtifactsList()) || [];
this.getRowsFromArtifacts(request, artifacts);
this.clearBanner();
});
const artifacts = (res && res.getArtifactsList()) || [];
await this.getRowsFromArtifacts(request, artifacts);
return '';
}

Expand Down Expand Up @@ -160,77 +163,78 @@ class ArtifactList extends Page<{}, ArtifactListState> {
* TODO: Replace once https://github.com/kubeflow/metadata/issues/73 is done.
* @param request
*/
private getRowsFromArtifacts(request: ListRequest, artifacts: Artifact[]): void {
private async getRowsFromArtifacts(request: ListRequest, artifacts: Artifact[]): Promise<void> {
const artifactTypesMap = new Map<number, ArtifactType>();
// TODO: Consider making an Api method for returning and caching types
Apis.getMetadataServiceClient().getArtifactTypes(
const {
response: res,
error: err,
} = await Apis.getMetadataServicePromiseClient().getArtifactTypes(
new GetArtifactTypesRequest(),
async (err, res) => {
if (err) {
this.showPageError(serviceErrorToString(err));
return;
}
);
if (err) {
this.showPageError(serviceErrorToString(err));
return;
}

((res && res.getArtifactTypesList()) || []).forEach(artifactType => {
artifactTypesMap.set(artifactType.getId()!, artifactType);
});
((res && res.getArtifactTypesList()) || []).forEach(artifactType => {
artifactTypesMap.set(artifactType.getId()!, artifactType);
});

try {
// TODO: When backend supports sending creation time back when we list
// artifacts, let's use it directly.
const artifactsWithCreationTimes = await Promise.all(
artifacts.map(async artifact => {
const artifactId = artifact.getId();
if (!artifactId) {
return { artifact };
}
try {
// TODO: When backend supports sending creation time back when we list
// artifacts, let's use it directly.
const artifactsWithCreationTimes = await Promise.all(
artifacts.map(async artifact => {
const artifactId = artifact.getId();
if (!artifactId) {
return { artifact };
}

return {
artifact,
creationTime: await getArtifactCreationTime(artifactId),
};
}),
);
return {
artifact,
creationTime: await getArtifactCreationTime(artifactId),
};
}),
);

const collapsedAndExpandedRows = groupRows(
artifactsWithCreationTimes
.map(({ artifact, creationTime }) => {
const typeId = artifact.getTypeId();
const type =
typeId && artifactTypesMap && artifactTypesMap.get(typeId)
? artifactTypesMap.get(typeId)!.getName()
: typeId;
return {
id: `${type}:${artifact.getId()}`, // Join with colon so we can build the link
otherFields: [
getResourceProperty(artifact, ArtifactProperties.PIPELINE_NAME) ||
getResourceProperty(artifact, ArtifactCustomProperties.WORKSPACE, true),
getResourceProperty(artifact, ArtifactProperties.NAME),
artifact.getId(),
type,
artifact.getUri(),
creationTime || '',
],
} as Row;
})
.filter(rowFilterFn(request))
.sort(rowCompareFn(request, this.state.columns)),
);
const collapsedAndExpandedRows = groupRows(
artifactsWithCreationTimes
.map(({ artifact, creationTime }) => {
const typeId = artifact.getTypeId();
const type =
typeId && artifactTypesMap && artifactTypesMap.get(typeId)
? artifactTypesMap.get(typeId)!.getName()
: typeId;
return {
id: `${type}:${artifact.getId()}`, // Join with colon so we can build the link
otherFields: [
getResourceProperty(artifact, ArtifactProperties.PIPELINE_NAME) ||
getResourceProperty(artifact, ArtifactCustomProperties.WORKSPACE, true),
getResourceProperty(artifact, ArtifactProperties.NAME),
artifact.getId(),
type,
artifact.getUri(),
creationTime || '',
],
} as Row;
})
.filter(rowFilterFn(request))
.sort(rowCompareFn(request, this.state.columns)),
);

this.setState({
artifacts,
expandedRows: collapsedAndExpandedRows.expandedRows,
rows: collapsedAndExpandedRows.collapsedRows,
});
} catch (err) {
if (err.message) {
this.showPageError(err.message, err);
} else {
this.showPageError('Unknown error', err);
}
}
},
);
this.setState({
artifacts,
expandedRows: collapsedAndExpandedRows.expandedRows,
rows: collapsedAndExpandedRows.collapsedRows,
});
} catch (err) {
if (err.message) {
this.showPageError(err.message, err);
} else {
this.showPageError('Unknown error', err);
}
}
}

/**
Expand Down
109 changes: 59 additions & 50 deletions frontend/src/pages/ExecutionList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,18 +117,22 @@ class ExecutionList extends Page<{}, ExecutionListState> {
}

private async reload(request: ListRequest): Promise<string> {
Apis.getMetadataServiceClient().getExecutions(new GetExecutionsRequest(), (err, res) => {
const {
error: err,
response: res,
} = await Apis.getMetadataServicePromiseClient().getExecutions(new GetExecutionsRequest());

if (err) {
// Code === 5 means no record found in backend. This is a temporary workaround.
// TODO: remove err.code !== 5 check when backend is fixed.
if (err && err.code !== 5) {
if (err.code !== 5) {
this.showPageError(serviceErrorToString(err));
return;
}
return '';
}

const executions = (res && res.getExecutionsList()) || [];
this.getRowsFromExecutions(request, executions);
this.clearBanner();
});
const executions = (res && res.getExecutionsList()) || [];
await this.getRowsFromExecutions(request, executions);
return '';
}

Expand All @@ -153,53 +157,58 @@ class ExecutionList extends Page<{}, ExecutionListState> {
* TODO: Replace once https://github.com/kubeflow/metadata/issues/73 is done.
* @param request
*/
private getRowsFromExecutions(request: ListRequest, executions: Execution[]): void {
private async getRowsFromExecutions(
request: ListRequest,
executions: Execution[],
): Promise<void> {
const executionTypesMap = new Map<number, ExecutionType>();
// TODO: Consider making an Api method for returning and caching types
Apis.getMetadataServiceClient().getExecutionTypes(
const {
error: err,
response: res,
} = await Apis.getMetadataServicePromiseClient().getExecutionTypes(
new GetExecutionTypesRequest(),
(err, res) => {
if (err) {
this.showPageError(serviceErrorToString(err));
return;
}

((res && res.getExecutionTypesList()) || []).forEach(executionType => {
executionTypesMap.set(executionType.getId()!, executionType);
});

const collapsedAndExpandedRows = groupRows(
executions
.map(execution => {
// Flattens
const typeId = execution.getTypeId();
const type =
typeId && executionTypesMap && executionTypesMap.get(typeId)
? executionTypesMap.get(typeId)!.getName()
: typeId;
return {
id: `${type}:${execution.getId()}`, // Join with colon so we can build the link
otherFields: [
getResourceProperty(execution, ExecutionProperties.PIPELINE_NAME) ||
getResourceProperty(execution, ExecutionCustomProperties.WORKSPACE, true),
getResourceProperty(execution, ExecutionProperties.COMPONENT_ID),
getResourceProperty(execution, ExecutionProperties.STATE),
execution.getId(),
type,
],
};
})
.filter(rowFilterFn(request))
.sort(rowCompareFn(request, this.state.columns)),
);

this.setState({
executions,
expandedRows: collapsedAndExpandedRows.expandedRows,
rows: collapsedAndExpandedRows.collapsedRows,
});
},
);

if (err) {
this.showPageError(serviceErrorToString(err));
return;
}

((res && res.getExecutionTypesList()) || []).forEach(executionType => {
executionTypesMap.set(executionType.getId()!, executionType);
});

const collapsedAndExpandedRows = groupRows(
executions
.map(execution => {
// Flattens
const typeId = execution.getTypeId();
const type =
typeId && executionTypesMap && executionTypesMap.get(typeId)
? executionTypesMap.get(typeId)!.getName()
: typeId;
return {
id: `${type}:${execution.getId()}`, // Join with colon so we can build the link
otherFields: [
getResourceProperty(execution, ExecutionProperties.PIPELINE_NAME) ||
getResourceProperty(execution, ExecutionCustomProperties.WORKSPACE, true),
getResourceProperty(execution, ExecutionProperties.COMPONENT_ID),
getResourceProperty(execution, ExecutionProperties.STATE),
execution.getId(),
type,
],
};
})
.filter(rowFilterFn(request))
.sort(rowCompareFn(request, this.state.columns)),
);

this.setState({
executions,
expandedRows: collapsedAndExpandedRows.expandedRows,
rows: collapsedAndExpandedRows.collapsedRows,
});
}

/**
Expand Down