Skip to content

Commit

Permalink
fix: Delete associated artifacts and metrics data while deleting a run
Browse files Browse the repository at this point in the history 
Fixes #37,#51
  • Loading branch information
@vivekratnavel
vivekratnavel committed Jan 20, 2019
1 parent e5563bc commit 284b787
Show file tree
Hide file tree
Showing 5 changed files with 160 additions and 37 deletions.
95 changes: 79 additions & 16 deletions web/src/components/Helpers/cells.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ class IdCell extends Component {
super(props);
this.state = {
showDeleteIcon: false,
showModal: false
showModal: false,
isDeleteInProgress: false
};
}

Expand Down Expand Up @@ -119,27 +120,89 @@ class IdCell extends Component {

_deleteHandler = (experimentId) => (e) => {
e.stopPropagation();
axios.delete('/api/v1/Runs/' + experimentId).then(response => {
if (response.status === 204) {
// Call callback function to update rows in the table
this.props.handleDataUpdate(experimentId);
toast.success(`Experiment run ${experimentId} was deleted successfully!`);
} else {
toast.error('An unknown error occurred!');
}
if (experimentId && !isNaN(experimentId)) {
this.setState({
showModal: false
isDeleteInProgress: true
});
}).catch(error => {
toast.error(parseServerError(error));
});
axios.get('/api/v1/Runs/' + experimentId, {
params: {
select: 'artifacts,metrics',
populate: 'metrics'
}
}).then(response => {
const runsResponse = response.data;
const deleteApis = [];
if(runsResponse.metrics && runsResponse.metrics.length) {
deleteApis.push(
axios.delete('/api/v1/Metrics/', {
params: {
query: JSON.stringify({
run_id: experimentId
})
}
}));
}
if (runsResponse.artifacts && runsResponse.artifacts.length) {
const chunksQuery = runsResponse.artifacts.map(file => {
return {files_id: file.file_id}
});
const filesQuery = runsResponse.artifacts.map(file => {
return {_id: file.file_id}
});
deleteApis.push(
axios.delete('/api/v1/Fs.chunks/', {
params: {
query: JSON.stringify({
$or: chunksQuery
})
}
}));
deleteApis.push(
axios.delete('/api/v1/Fs.files/', {
params: {
query: JSON.stringify({
$or: filesQuery
})
}
}));
}
deleteApis.push(
axios.delete('/api/v1/Runs/' + experimentId)
);

axios.all(deleteApis).then(axios.spread((...deleteResponses) => {
if (deleteResponses.every(response => response.status === 204)) {
// Call callback function to update rows in the table
this.props.handleDataUpdate(experimentId);
toast.success(`Experiment run ${experimentId} was deleted successfully!`);
} else {
toast.error('An unknown error occurred!');
}
this.setState({
isDeleteInProgress: false,
showModal: false
});
})).catch(error => {
toast.error(parseServerError(error));
this.setState({
isDeleteInProgress: false
});
});
}).catch(error => {
toast.error(parseServerError(error));
this.setState({
isDeleteInProgress: false
});
});
}
};

render() {
const {data, rowIndex, columnKey, handleDataUpdate, ...props} = this.props;
const {showDeleteIcon, showModal} = this.state;
const {showDeleteIcon, showModal, isDeleteInProgress} = this.state;
const deleteIcon = showDeleteIcon ? <Glyphicon glyph="trash" className="delete-icon" onClick={this._onDeleteClickHandler}/> : null;
const experimentId = data.getObjectAt(rowIndex)[columnKey];
const deleteGlyph = isDeleteInProgress ? <i className="glyphicon glyphicon-refresh glyphicon-refresh-animate"/> : <Glyphicon glyph="trash" />;
return (
<div>
<Cell {...props} onMouseEnter={this._mouseEnterHandler} onMouseLeave={this._mouseLeaveHandler}>
Expand All @@ -156,8 +219,8 @@ class IdCell extends Component {
</ModalBody>
<ModalFooter>
<Button test-attr="close-btn" onClick={this._closeModalHandler}>Cancel</Button>
<Button test-attr="delete-btn" bsStyle="danger" onClick={this._deleteHandler(experimentId)}>
<Glyphicon glyph="trash" /> Delete
<Button test-attr="delete-btn" bsStyle="danger" disabled={isDeleteInProgress} onClick={this._deleteHandler(experimentId)}>
{deleteGlyph} Delete
</Button>
</ModalFooter>
</Modal>
Expand Down
78 changes: 69 additions & 9 deletions web/src/components/Helpers/cells.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,30 +157,90 @@ describe('Cells', () => {
const event = {
stopPropagation: jest.fn()
};
const artifactsResponse = [{"file_id":"5c41711ea9eee738179295aa","name":"result.pickle"},{"file_id":"5c41711ea9eee738179295ac","name":"test.svg"},{"file_id":"5c41711ea9eee738179295ae","name":"output.png"}];
const metricsResponse = [{"_id":"5a2179f9fccf1dcc0ee39e63","name":"finetune.train.loss","run_id":54,"steps":[0,1,2],"timestamps":["2017-12-01T15:49:06.412Z","2017-12-01T15:51:27.910Z","2017-12-01T15:53:49.750Z"],"values":[0.9302947769183239,0.5418723183750066,0.505903509070725]},{"_id":"5a217a03fccf1dcc0ee39e74","name":"finetune.val.loss","run_id":54,"steps":[0,1,2,3],"timestamps":["2017-12-01T15:49:16.322Z","2017-12-01T15:51:37.849Z","2017-12-01T15:53:59.725Z"],"values":[0.6144198719169135,0.34360378449377804,0.4291475112023561]}];
toast.error = jest.fn();
beforeEach(() => {
wrapper.find('FixedDataTableCellDefault').simulate('mouseEnter');
wrapper.find('.delete-icon').at(0).simulate('click', event);
wrapper.find('[test-attr="delete-btn"]').at(1).simulate('click', event);
});

it('success', () => {
mockAxios.mockResponse({status: 204});
describe('success', () => {
it('for metrics', async () => {
mockAxios.mockResponse({status: 200, data: {"_id":54,"artifacts":[],"metrics": metricsResponse}});

expect(wrapper.state().isDeleteInProgress).toBeTruthy();
expect(mockAxios.delete).toHaveBeenCalledTimes(2);
mockAxios.mockResponse({status: 204});
mockAxios.mockResponse({status: 204});
await tick();

expect(wrapper.state().isDeleteInProgress).toBeFalsy();
expect(dataUpdateHandler).toHaveBeenCalledWith(data.getObjectAt(rowIndex)._id);
});

it('for artifacts', async () => {
mockAxios.mockResponse({status: 200, data: {"_id":54,"artifacts": artifactsResponse,"metrics": []}});

expect(mockAxios.delete).toHaveBeenCalledTimes(3);
mockAxios.mockResponse({status: 204});
mockAxios.mockResponse({status: 204});
mockAxios.mockResponse({status: 204});
await tick();

expect(dataUpdateHandler).toHaveBeenCalledWith(data.getObjectAt(rowIndex)._id);
});

it('for both artifacts and metrics', async () => {
mockAxios.mockResponse({status: 200, data: {"_id":54,"artifacts": artifactsResponse,"metrics": metricsResponse}});

expect(dataUpdateHandler).toHaveBeenCalledWith(data.getObjectAt(rowIndex)._id);
expect(mockAxios.delete).toHaveBeenCalledTimes(4);
mockAxios.mockResponse({status: 204});
mockAxios.mockResponse({status: 204});
mockAxios.mockResponse({status: 204});
mockAxios.mockResponse({status: 204});
await tick();

expect(dataUpdateHandler).toHaveBeenCalledWith(data.getObjectAt(rowIndex)._id);
});

it('for no artifacts or metrics', async () => {
mockAxios.mockResponse({status: 200, data: {"_id":54,"artifacts": [],"metrics": []}});

expect(mockAxios.delete).toHaveBeenCalledTimes(1);
mockAxios.mockResponse({status: 204});
await tick();

expect(dataUpdateHandler).toHaveBeenCalledWith(data.getObjectAt(rowIndex)._id);
});
});

it('unknown error', () => {
mockAxios.mockResponse({status: 200});
it('unknown error', async () => {
mockAxios.mockResponse({status: 200, data:{"_id":54,"artifacts":[],"metrics": metricsResponse}});
mockAxios.mockResponse({status: 204});
mockAxios.mockResponse({status: 400});
await tick();

expect(toast.error).toHaveBeenCalledWith(`An unknown error occurred!`);
});

it('error', () => {
const errResponse = {status: 500, message:'unknown error'};
mockAxios.mockError(errResponse);
describe('error', () => {
it('for get', () => {
const errResponse = {status: 500, message:'unknown error'};
mockAxios.mockError(errResponse);

expect(toast.error).toHaveBeenCalledWith(`Error: ${errResponse.message}`);
});

it('for delete calls', async () => {
mockAxios.mockResponse({status: 200, data: {"_id":54,"artifacts": [],"metrics": []}});
const errResponse = {status: 500, message:'unknown error'};
mockAxios.mockError(errResponse);
await tick();

expect(toast.error).toHaveBeenCalledWith(`Error: ${errResponse.message}`);
expect(toast.error).toHaveBeenCalledWith(`Error: ${errResponse.message}`);
});
});
});
});
Expand Down
14 changes: 7 additions & 7 deletions web/src/components/MetricsPlotView/metricsPlotView.js
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React, {Component} from 'react';
import { SCALE_VALUE, scaleValues, xAxisValues } from '../../constants/drillDownView.constants';
import { SCALE_VALUE, SCALE_VALUES, X_AXIS_VALUES } from '../../constants/drillDownView.constants';
import { capitalize } from '../Helpers/utils';
import Plot from 'react-plotly.js';
import PropTypes from 'prop-types'
Expand All @@ -26,8 +26,8 @@ class MetricsPlotView extends Component {
super(props);
this.state = {
selectedMetricNames: [],
selectedXAxis: xAxisValues[0],
selectedYAxis: scaleValues[0],
selectedXAxis: X_AXIS_VALUES[0],
selectedYAxis: SCALE_VALUES[0],
plotWidth: DEFAULT_PLOT_WIDTH,
plotHeight: DEFAULT_PLOT_HEIGHT
};
Expand Down Expand Up @@ -86,8 +86,8 @@ class MetricsPlotView extends Component {
const selectedMetricNames = defaultSelection.selectedMetricNames ? defaultSelection.selectedMetricNames.filter(name => metricNames.includes(name)) : [];
this.setState({
selectedMetricNames,
selectedXAxis: defaultSelection.selectedXAxis || '',
selectedYAxis: defaultSelection.selectedYAxis || '',
selectedXAxis: defaultSelection.selectedXAxis || X_AXIS_VALUES[0],
selectedYAxis: defaultSelection.selectedYAxis || SCALE_VALUES[0],
plotWidth: defaultSelection.plotWidth || DEFAULT_PLOT_WIDTH,
plotHeight: defaultSelection.plotHeight || DEFAULT_PLOT_HEIGHT
});
Expand Down Expand Up @@ -161,7 +161,7 @@ class MetricsPlotView extends Component {
</div>
<h4>X-Axis Type</h4>
<div id="plot-x-axis-types">
{xAxisValues.map( (value, i) => {
{X_AXIS_VALUES.map( (value, i) => {
return (
<div key={"XAxisPlot" + runId + i} className="radio">
<label>
Expand All @@ -174,7 +174,7 @@ class MetricsPlotView extends Component {
</div>
<h4>Y-Axis Type</h4>
<div id="plot-y-axis-types">
{scaleValues.map( (value, i) => {
{SCALE_VALUES.map( (value, i) => {
return (
<div key={"YAxisPlot" + runId + i} className="radio">
<label>
Expand Down
6 changes: 3 additions & 3 deletions web/src/components/MetricsPlotView/metricsPlotView.test.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import { MetricsPlotView } from './metricsPlotView';
import mockAxios from 'jest-mock-axios';
import { X_AXIS_VALUE, SCALE_VALUE } from '../../constants/drillDownView.constants';
import {X_AXIS_VALUE, SCALE_VALUE, X_AXIS_VALUES, SCALE_VALUES} from '../../constants/drillDownView.constants';
import keyCode from 'rc-util/lib/KeyCode';
import { LocalStorageMock } from '../../../config/jest/localStorageMock';

Expand Down Expand Up @@ -56,8 +56,8 @@ describe('MetricsPlotView', () => {
wrapper.instance()._setDefaultSelection();

expect(wrapper.state().selectedMetricNames).toHaveLength(0);
expect(wrapper.state().selectedXAxis).toEqual('');
expect(wrapper.state().selectedYAxis).toEqual('');
expect(wrapper.state().selectedXAxis).toEqual(X_AXIS_VALUES[0]);
expect(wrapper.state().selectedYAxis).toEqual(SCALE_VALUES[0]);
expect(wrapper.state().plotWidth).toEqual(800);
expect(wrapper.state().plotHeight).toEqual(400);
// reset localStorage
Expand Down
4 changes: 2 additions & 2 deletions web/src/constants/drillDownView.constants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,5 @@ export const SCALE_VALUE = {
LOGARITHMIC:'logarithmic'
};

export const xAxisValues = [X_AXIS_VALUE.STEPS, X_AXIS_VALUE.TIME];
export const scaleValues = [SCALE_VALUE.LINEAR, SCALE_VALUE.LOGARITHMIC];
export const X_AXIS_VALUES = [X_AXIS_VALUE.STEPS, X_AXIS_VALUE.TIME];
export const SCALE_VALUES = [SCALE_VALUE.LINEAR, SCALE_VALUE.LOGARITHMIC];

0 comments on commit 284b787

Please sign in to comment.