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
21 changes: 20 additions & 1 deletion frontend/src/components/worksheets/BundleDetail/BundleDetail.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import BundleActions from './BundleActions';

const BundleDetail = ({
uuid,
// Callback on metadata change.
bundleInfoFromRow,
bundleMetadataChanged,
contentExpanded,
onOpen,
Expand Down Expand Up @@ -204,6 +204,25 @@ const BundleDetail = ({
}
};

/**
* This helper syncs the state info that is used to render the bundle row
* with the state info that is passed down into the bundle detail sidebar.
*
* This enables the bundle row and the bundle detail sidebar to show the
* exact same state information.
*/
const syncBundleStateInfo = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This only is called once, so why not just inline?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I feel like wrapping the logic in a named function makes it easier to figure out for someone else who might stumble across the code later.

bundleInfo.state = bundleInfoFromRow.state;
bundleInfo.state_details = bundleInfoFromRow.state_details;
bundleInfo.metadata.time_preparing = bundleInfoFromRow.metadata.time_preparing;
bundleInfo.metadata.time_running = bundleInfoFromRow.metadata.time_running;
bundleInfo.metadata.time = bundleInfoFromRow.metadata.time;
};

if (bundleInfoFromRow && bundleInfo) {
syncBundleStateInfo();
}

if (!bundleInfo) {
if (metadataErrors.length) {
return <ErrorMessage message='Error: Bundle Unavailable' />;
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,36 +20,45 @@ import BundleFieldRow from './BundleFieldRow';
class BundleStateTable extends React.Component {
constructor(props) {
super(props);
const states = this.getStates();
this.state = {
states,
};
const bundleType = this.props.bundle.bundle_type.value;
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 { bundle } = this.props;
const timePreparing = bundle.time_preparing?.value;
const timeRunning = bundle.time_running?.value;
const time = bundle.time?.value;

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;
}
}
}

Expand All @@ -53,8 +68,9 @@ class BundleStateTable extends React.Component {
const stateDetails = bundle.state_details?.value;
const currentState = bundle.state.value;
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 @@ -306,6 +306,7 @@ class BundleRow extends Component {
>
<BundleDetail
uuid={bundleInfo.uuid}
bundleInfoFromRow={bundleInfo}
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