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.

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

Sync bundle row state and bundle detail state #4246

Merged
merged 11 commits into from
Sep 9, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const BundleDetail = ({
sidebarExpanded,
hideBundlePageLink,
showBorder,
stateInfoFromWorksheet,
}) => {
const [bundleInfo, setBundleInfo] = useState(null);
const [contentType, setContentType] = useState(null);
Expand Down Expand Up @@ -233,6 +234,7 @@ const BundleDetail = ({
sidebar={
<BundleDetailSideBar
bundleInfo={bundleInfo}
stateInfoFromWorksheet={stateInfoFromWorksheet}
onUpdate={onUpdate}
onMetaDataChange={mutateMetadata}
expanded={sidebarExpanded}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';
import { withStyles } from '@material-ui/core';
import { formatBundle } from '../../../util/worksheet_utils';
import { formatBundle, getStateInfo } from '../../../util/worksheet_utils';
import { FINAL_BUNDLE_STATES } from '../../../constants';
import CollapseButton from '../../CollapseButton';
import NewWindowLink from '../../NewWindowLink';
Expand Down Expand Up @@ -49,15 +49,16 @@ class BundleDetailSideBar extends React.Component {

render() {
const { bundleInfo, classes, hidePageLink, onUpdate, onMetaDataChange } = this.props;
const { expandPermissons, showMoreDetail } = this.state;
const stateInfo = this.props.stateInfoFromWorksheet || getStateInfo(bundleInfo);
const inFinalState = FINAL_BUNDLE_STATES.includes(stateInfo.state);
const bundle = formatBundle(bundleInfo);
const bundleType = bundle.bundle_type.value;
const uuid = bundle.uuid.value;
const time = bundle.time?.value;
const exclusions = bundle.exclude_patterns;
const state = bundle.state.value;
const inFinalState = FINAL_BUNDLE_STATES.includes(state);

const expandPermissons = this.state.expandPermissons;
const showMoreDetail = this.state.showMoreDetail;
const showPageLink = !hidePageLink;
const showOwner = !bundle.is_anonymous.value;
const showTime = inFinalState && (time || bundle.request_time?.editable);
Expand All @@ -73,7 +74,7 @@ class BundleDetailSideBar extends React.Component {
<NewWindowLink className={classes.pageLink} href={`/bundles/${uuid}`} />
)}
<BundleFieldTable>
<BundleStateRow bundle={bundle} />
<BundleStateRow stateInfo={stateInfo} />
<BundleFieldRow
label='UUID'
description="Click the copy icon to copy the bundle's full UUID."
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import React from 'react';
import { withStyles } from '@material-ui/core/styles';
import { FINAL_BUNDLE_STATES } from '../../../../constants';
import {
FINAL_BUNDLE_STATES,
RUN_BUNDLE_STATES,
UPLOADED_BUNDLE_STATES,
MAKE_BUNDLE_STATES,
OFFLINE_STATE,
} from '../../../../constants';
import BundleStateBox from '../BundleStateBox';
import BundleFieldRow from './BundleFieldRow';

Expand All @@ -14,47 +20,54 @@ import BundleFieldRow from './BundleFieldRow';
class BundleStateTable extends React.Component {
constructor(props) {
super(props);
const states = this.getStates();
this.state = {
states,
};
const bundleType = this.props.stateInfo.bundleType;
const states = this.getStates(bundleType);
this.state = { states };
}

getStates() {
const bundleType = this.props.bundle.bundle_type.value;
getStates(bundleType) {
if (bundleType === 'run') {
return ['created', 'staged', 'starting', 'preparing', 'running', 'finalizing', 'ready'];
return RUN_BUNDLE_STATES;
}
if (bundleType === 'dataset') {
return ['created', 'uploading', 'ready'];
return UPLOADED_BUNDLE_STATES;
}
if (bundleType === 'make') {
return ['created', 'making', 'ready'];
return MAKE_BUNDLE_STATES;
}
return [];
}

showTime(time) {
return time && time !== '<none>' && time !== '0s';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we should be able to access the raw value rather than the rendered one to avoid these rather brittle comparisons, so we could test whether time == null or something...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will update the renderFormat helper to allow for this 👍

}

getTime(state) {
const bundle = this.props.bundle;
let time;
if (state === 'preparing') {
time = bundle.time_preparing?.value;
} else if (state === 'running') {
time = bundle.time_running?.value || bundle.time?.value;
const { showTime } = this;
const { stateInfo } = this.props;
const { timePreparing, timeRunning, time } = stateInfo;
if (state === 'preparing' && showTime(timePreparing)) {
return timePreparing;
}
if (time && time !== '0s') {
return time;
if (state === 'running') {
if (showTime(timeRunning)) {
return timeRunning;
}
if (showTime(time)) {
return time;
}
}
}

render() {
const { bundle, classes } = this.props;
const { classes, stateInfo } = this.props;
const { states } = this.state;
const stateDetails = bundle.state_details?.value;
const currentState = bundle.state.value;
const stateDetails = stateInfo.stateDetails;
const currentState = stateInfo.state;
const inFinalState = FINAL_BUNDLE_STATES.includes(currentState);
const inOfflineState = currentState === OFFLINE_STATE;

if (inFinalState) {
if (inFinalState || inOfflineState) {
return (
<BundleFieldRow
label='State'
Expand Down
14 changes: 7 additions & 7 deletions frontend/src/components/worksheets/Worksheet/Worksheet.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
LOCAL_STORAGE_WORKSHEET_WIDTH,
DIALOG_TYPES,
AUTO_HIDDEN_DURATION,
FINAL_BUNDLE_STATES,
} from '../../../constants';
import WorksheetTerminal from '../WorksheetTerminal';
import Loading from '../../Loading';
Expand Down Expand Up @@ -1224,13 +1225,12 @@ class Worksheet extends React.Component {
if (items[i].bundles_spec) {
for (var j = 0; j < items[i].bundles_spec.bundle_infos.length; j++) {
var bundle_info = items[i].bundles_spec.bundle_infos[j];
if (bundle_info.bundle_type === 'run') {
if (bundle_info.state !== 'ready' && bundle_info.state !== 'failed') {
updatingBundleUuids[bundle_info.uuid] = true;
} else {
if (bundle_info.uuid in updatingBundleUuids)
delete updatingBundleUuids[bundle_info.uuid];
}
const inFinalState = FINAL_BUNDLE_STATES.includes(bundle_info.state);
if (!inFinalState) {
updatingBundleUuids[bundle_info.uuid] = true;
} else {
if (bundle_info.uuid in updatingBundleUuids)
delete updatingBundleUuids[bundle_info.uuid];
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import BundleDetail from '../../BundleDetail';
import TextEditorItem from '../TextEditorItem';
import SchemaItem from '../SchemaItem';
import { DEFAULT_SCHEMA_ROWS } from '../../../../constants';
import { getStateInfo } from '../../../../util/worksheet_utils';

// The approach taken in this design is to hack the HTML `Table` element by using one `TableBody` for each `BundleRow`.
// We need the various columns to be aligned for all `BundleRow` within a `Table`, therefore using `div` is not an
Expand Down Expand Up @@ -117,6 +118,7 @@ class BundleRow extends Component {
focusIndex,
ws,
} = this.props;
const stateInfoFromWorksheet = getStateInfo(bundleInfo);
leilenah marked this conversation as resolved.
Show resolved Hide resolved
const rowItems = { ...item, ...bundleInfoUpdates };
var baseUrl = this.props.url;
var uuid = this.props.uuid;
Expand Down Expand Up @@ -306,6 +308,7 @@ class BundleRow extends Component {
>
<BundleDetail
uuid={bundleInfo.uuid}
stateInfoFromWorksheet={stateInfoFromWorksheet}
bundleMetadataChanged={this.props.reloadWorksheet}
onUpdate={this.receiveBundleInfoUpdates}
onClose={() => {
Expand Down
18 changes: 17 additions & 1 deletion frontend/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,23 @@ export const BUNDLE_STATES: String[] = [
];

// All possible final bundle states
export const FINAL_BUNDLE_STATES = ['ready', 'failed', 'killed', 'worker_offline'];
export const FINAL_BUNDLE_STATES = ['ready', 'failed', 'killed'];

export const RUN_BUNDLE_STATES = [
'created',
'staged',
'starting',
'preparing',
'running',
'finalizing',
'ready',
];

export const UPLOADED_BUNDLE_STATES = ['created', 'uploading', 'ready'];

export const MAKE_BUNDLE_STATES = ['created', 'making', 'ready'];

export const OFFLINE_STATE = 'worker_offline';

// Autofill types for schemas.
export const DEFAULT_POST_PROCESSOR = {
Expand Down
25 changes: 25 additions & 0 deletions frontend/src/util/worksheet_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -416,3 +416,28 @@ export function formatBundle(bundle) {

return result;
}

/**
* Takes in bundle information and returns an object containing all bundle
* information related to the bundle's state.
*
* @param {object} bundleInfo
* @returns {object} stateInfo
*/
export function getStateInfo(bundleInfo = {}) {
const state = bundleInfo.state;
const stateDetails = bundleInfo.state_details;
const bundleType = bundleInfo.bundle_type;
const metadata = bundleInfo.metadata;
const timePreparing = renderFormat(metadata.time_preparing, 'duration');
const timeRunning = renderFormat(metadata.time_running, 'duration');
const time = renderFormat(metadata.time, 'duration');
return {
state,
stateDetails,
bundleType,
timePreparing,
timeRunning,
time,
};
}