[UI] Add serverside order_by support (mlflow#1455)

mlflow/entities/run_info.py

@@ -24,6 +24,12 @@ class searchable_attribute(property):
     pass
 
 
+class orderable_attribute(property):
+    # Wrapper class over property to designate some of the properties as orderable
+    # run attributes
+    pass
+
+
 class RunInfo(_MLflowObject):
     """
     Metadata about a run.
@@ -99,7 +105,7 @@ def status(self):
         """
         return self._status
 
-    @property
+    @orderable_attribute
     def start_time(self):
         """Start time of the run, in number of milliseconds since the UNIX epoch."""
         return self._start_time
@@ -149,3 +155,10 @@ def from_proto(cls, proto):
     def get_searchable_attributes(cls):
         return sorted([p for p in cls.__dict__
                        if isinstance(getattr(cls, p), searchable_attribute)])
+
+    @classmethod
+    def get_orderable_attributes(cls):
+        # Note that all searchable attributes are also orderable.
+        return sorted([p for p in cls.__dict__
+                       if isinstance(getattr(cls, p), searchable_attribute) or
+                       isinstance(getattr(cls, p), orderable_attribute)])

mlflow/server/js/src/Actions.js

@@ -79,14 +79,15 @@ export const restoreRunApi = (runUuid, id = getUUID()) => {
 };
 
 export const SEARCH_RUNS_API = 'SEARCH_RUNS_API';
-export const searchRunsApi = (experimentIds, filter, runViewType, id = getUUID()) => {
+export const searchRunsApi = (experimentIds, filter, runViewType, orderBy, id = getUUID()) => {
   return {
     type: SEARCH_RUNS_API,
     payload: wrapDeferred(MlflowService.searchRuns, {
       experiment_ids: experimentIds,
       filter: filter,
       run_view_type: runViewType,
       max_results: SEARCH_MAX_RESULTS + 1,
+      order_by: orderBy,
     }),
     meta: { id: id },
   };

mlflow/server/js/src/components/BaggedCell.js

@@ -2,6 +2,7 @@ import React, { PureComponent } from 'react';
 import PropTypes from 'prop-types';
 import { Dropdown, MenuItem } from 'react-bootstrap';
 import classNames from 'classnames';
+import ExperimentViewUtil from "./ExperimentViewUtil";
 import ExperimentRunsSortToggle from './ExperimentRunsSortToggle';
 import EmptyIfClosedMenu from './EmptyIfClosedMenu';
 
@@ -16,16 +17,17 @@ export default class BaggedCell extends PureComponent {
   static propTypes = {
     keyName: PropTypes.string.isRequired,
     value: PropTypes.string.isRequired,
-    setSortByHandler: PropTypes.func.isRequired,
+    onSortBy: PropTypes.func.isRequired,
     isParam: PropTypes.bool.isRequired,
     isMetric: PropTypes.bool.isRequired,
     onRemoveBagged: PropTypes.func.isRequired,
     sortIcon: PropTypes.node,
   };
 
   render() {
-    const { keyName, value, setSortByHandler, isParam, isMetric, onRemoveBagged,
-      sortIcon } = this.props;
+    const { keyName, value, onSortBy, isParam, onRemoveBagged, sortIcon } = this.props;
+    const keyType = (isParam ? "params" : "metrics");
+    const canonicalKey = ExperimentViewUtil.makeCanonicalKey(keyType, keyName);
     const cellClass = classNames("metric-param-content", "metric-param-cell", "BaggedCell");
     return (
       <span
@@ -54,13 +56,13 @@ export default class BaggedCell extends PureComponent {
         <EmptyIfClosedMenu className="mlflow-menu" bsRole="menu">
           <MenuItem
             className="mlflow-menu-item"
-            onClick={() => setSortByHandler(isMetric, isParam, keyName, true)}
+            onClick={() => onSortBy(canonicalKey, true)}
           >
             Sort ascending
           </MenuItem>
           <MenuItem
             className="mlflow-menu-item"
-            onClick={() => setSortByHandler(isMetric, isParam, keyName, false)}
+            onClick={() => onSortBy(canonicalKey, false)}
           >
             Sort descending
           </MenuItem>

mlflow/server/js/src/components/ExperimentPage.js

@@ -16,7 +16,7 @@ import { withRouter } from 'react-router-dom';
 
 export const LIFECYCLE_FILTER = { ACTIVE: 'Active', DELETED: 'Deleted' };
 
-class ExperimentPage extends Component {
+export class ExperimentPage extends Component {
   constructor(props) {
     super(props);
     this.onSearch = this.onSearch.bind(this);
@@ -28,13 +28,16 @@ class ExperimentPage extends Component {
         paramKeyFilterString: urlState.params === undefined ? "" : urlState.params,
         metricKeyFilterString: urlState.metrics === undefined ? "" : urlState.metrics,
         searchInput: urlState.search === undefined ? "" : urlState.search,
+        orderByKey: urlState.orderByKey === undefined ? null : urlState.orderByKey,
+        orderByAsc: urlState.orderByAsc === undefined ? true : urlState.orderByAsc === "true",
       },
     };
   }
 
   static propTypes = {
     experimentId: PropTypes.number.isRequired,
-    dispatchSearchRuns: PropTypes.func.isRequired,
+    getExperimentApi: PropTypes.func.isRequired,
+    searchRunsApi: PropTypes.func.isRequired,
     history: PropTypes.object.isRequired,
     location: PropTypes.object,
   };
@@ -76,37 +79,83 @@ class ExperimentPage extends Component {
         lastExperimentId: props.experimentId,
         lifecycleFilter: LIFECYCLE_FILTER.ACTIVE,
       };
-      props.dispatch(getExperimentApi(props.experimentId, newState.getExperimentRequestId));
-      props.dispatch(searchRunsApi(
+      props.getExperimentApi(props.experimentId, newState.getExperimentRequestId);
+      const orderBy = ExperimentPage.getOrderByExpr(newState.persistedState.orderByKey,
+        newState.persistedState.orderByAsc);
+      props.searchRunsApi(
         [props.experimentId],
         newState.persistedState.searchInput,
         lifecycleFilterToRunViewType(newState.lifecycleFilter),
-        newState.searchRunsRequestId));
+        orderBy,
+        newState.searchRunsRequestId);
       return newState;
     }
     return null;
   }
 
-  onSearch(paramKeyFilterString, metricKeyFilterString, searchInput, lifecycleFilterInput) {
+  onSearch(
+      paramKeyFilterString,
+      metricKeyFilterString,
+      searchInput,
+      lifecycleFilterInput,
+      orderByKey,
+      orderByAsc) {
     this.setState({
       persistedState: new ExperimentPagePersistedState({
         paramKeyFilterString,
         metricKeyFilterString,
         searchInput,
+        orderByKey,
+        orderByAsc,
       }).toJSON(),
       lifecycleFilter: lifecycleFilterInput,
     });
-    const searchRunsRequestId = this.props.dispatchSearchRuns(
-      this.props.experimentId, searchInput, lifecycleFilterInput);
+
+    const orderBy = ExperimentPage.getOrderByExpr(orderByKey, orderByAsc);
+    const searchRunsRequestId = getUUID();
+    this.props.searchRunsApi([this.props.experimentId], searchInput,
+      lifecycleFilterToRunViewType(lifecycleFilterInput), orderBy, searchRunsRequestId);
     this.setState({ searchRunsRequestId });
     this.updateUrlWithSearchFilter({
-      params: paramKeyFilterString,
-      metrics: metricKeyFilterString,
-      search: searchInput,
+      paramKeyFilterString,
+      metricKeyFilterString,
+      searchInput,
+      orderByKey,
+      orderByAsc,
     });
   }
 
-  updateUrlWithSearchFilter(state) {
+  static getOrderByExpr(orderByKey, orderByAsc) {
+    let orderBy = [];
+    if (orderByKey) {
+      if (orderByAsc) {
+        orderBy = [orderByKey + " ASC"];
+      } else {
+        orderBy = [orderByKey + " DESC"];
+      }
+    }
+    return orderBy;
+  }
+
+  updateUrlWithSearchFilter(
+      {paramKeyFilterString, metricKeyFilterString, searchInput, orderByKey, orderByAsc}) {
+    const state = {};
+    if (paramKeyFilterString) {
+      state['params'] = paramKeyFilterString;
+    }
+    if (metricKeyFilterString) {
+      state['metrics'] = metricKeyFilterString;
+    }
+    if (searchInput) {
+      state['search'] = searchInput;
+    }
+    if (orderByKey) {
+      state['orderByKey'] = orderByKey;
+    }
+    // orderByAsc defaults to true, so only encode it if it is false.
+    if (orderByAsc === false) {
+      state['orderByAsc'] = orderByAsc;
+    }
     const newUrl = `/experiments/${this.props.experimentId}` +
       `/s?${Utils.getSearchUrlFromState(state)}`;
     if (newUrl !== (this.props.history.location.pathname
@@ -140,6 +189,7 @@ class ExperimentPage extends Component {
             if (getExperimentRequest.active) {
               return <Spinner/>;
             }
+
             return <ExperimentView
               paramKeyFilter={new KeyFilter(this.state.persistedState.paramKeyFilterString)}
               metricKeyFilter={new KeyFilter(this.state.persistedState.metricKeyFilterString)}
@@ -150,6 +200,8 @@ class ExperimentPage extends Component {
               searchRunsError={searchRunsError}
               searchInput={this.state.persistedState.searchInput}
               isLoading={isLoading && !searchRunsError}
+              orderByKey={this.state.persistedState.orderByKey}
+              orderByAsc={this.state.persistedState.orderByAsc}
             />;
           }}
         </RequestStateWrapper>
@@ -162,16 +214,9 @@ class ExperimentPage extends Component {
   }
 }
 
-const mapDispatchToProps = (dispatch) => {
-  return {
-    dispatch,
-    dispatchSearchRuns: (experimentId, filter, lifecycleFilterInput) => {
-      const requestId = getUUID();
-      dispatch(searchRunsApi([experimentId], filter,
-        lifecycleFilterToRunViewType(lifecycleFilterInput), requestId));
-      return requestId;
-    }
-  };
+const mapDispatchToProps = {
+  getExperimentApi,
+  searchRunsApi,
 };
 
 const lifecycleFilterToRunViewType = (lifecycleFilter) => {

mlflow/server/js/src/components/ExperimentPage.test.js

@@ -0,0 +1,105 @@
+import React from 'react';
+import qs from 'qs';
+import { shallow } from 'enzyme';
+import { ExperimentPage } from './ExperimentPage';
+import { ViewType } from '../sdk/MlflowEnums';
+
+
+const BASE_PATH = "/experiments/17/s";
+const EXPERIMENT_ID = 17;
+
+let searchRunsApi;
+let getExperimentApi;
+let history;
+let location;
+
+beforeEach(() => {
+  searchRunsApi = jest.fn();
+  getExperimentApi = jest.fn();
+  location = {};
+
+  history = {};
+  history.location = {};
+  history.location.pathname = BASE_PATH;
+  history.location.search = "";
+  history.push = jest.fn();
+});
+
+const getExperimentPageMock = () => {
+  return shallow(<ExperimentPage
+    experimentId={EXPERIMENT_ID}
+    searchRunsApi={searchRunsApi}
+    getExperimentApi={getExperimentApi}
+    history={history}
+    location={location}
+  />);
+};
+
+function expectSearchState(historyEntry, state) {
+  const expectedPrefix = BASE_PATH + "?";
+  expect(historyEntry.startsWith(expectedPrefix)).toBe(true);
+  const search = historyEntry.substring(expectedPrefix.length);
+  const parsedHistory = qs.parse(search);
+  expect(parsedHistory).toEqual(state);
+}
+
+test('URL is empty for blank search', () => {
+  const wrapper = getExperimentPageMock();
+  wrapper.instance().onSearch("", "", "", "Active", null, true);
+  expectSearchState(history.push.mock.calls[0][0], {});
+  const searchRunsCall = searchRunsApi.mock.calls[1];
+  expect(searchRunsCall[0]).toEqual([EXPERIMENT_ID]);
+  expect(searchRunsCall[1]).toEqual("");
+  expect(searchRunsCall[2]).toEqual(ViewType.ACTIVE_ONLY);
+  expect(searchRunsCall[3]).toEqual([]);
+});
+
+test('URL can encode a complete search', () => {
+  const wrapper = getExperimentPageMock();
+  wrapper.instance().onSearch("key_filter", "metric0, metric1", "metrics.metric0 > 3",
+    "Deleted", null, true);
+  expectSearchState(history.push.mock.calls[0][0], {
+    "metrics": "metric0, metric1",
+    "params": "key_filter",
+    "search": "metrics.metric0 > 3"
+  });
+  const searchRunsCall = searchRunsApi.mock.calls[1];
+  expect(searchRunsCall[1]).toEqual("metrics.metric0 > 3");
+  expect(searchRunsCall[2]).toEqual(ViewType.DELETED_ONLY);
+});
+
+test('URL can encode order_by', () => {
+  const wrapper = getExperimentPageMock();
+  wrapper.instance().onSearch("key_filter", "metric0, metric1", "",
+    "Active", "my_key", false);
+  expectSearchState(history.push.mock.calls[0][0], {
+    "metrics": "metric0, metric1",
+    "params": "key_filter",
+    "orderByKey": "my_key",
+    "orderByAsc": "false",
+  });
+  const searchRunsCall = searchRunsApi.mock.calls[1];
+  expect(searchRunsCall[1]).toEqual("");
+  expect(searchRunsCall[3]).toEqual(["my_key DESC"]);
+});
+
+test('Loading state without any URL params', () => {
+  const wrapper = getExperimentPageMock();
+  const state = wrapper.instance().state;
+  expect(state.persistedState.paramKeyFilterString).toEqual("");
+  expect(state.persistedState.metricKeyFilterString).toEqual("");
+  expect(state.persistedState.searchInput).toEqual("");
+  expect(state.persistedState.orderByKey).toBe(null);
+  expect(state.persistedState.orderByAsc).toEqual(true);
+});
+
+test('Loading state with all URL params', () => {
+  location.search = "params=a&metrics=b&search=c&orderByKey=d&orderByAsc=false";
+  const wrapper = getExperimentPageMock();
+  const state = wrapper.instance().state;
+  expect(state.persistedState.paramKeyFilterString).toEqual("a");
+  expect(state.persistedState.metricKeyFilterString).toEqual("b");
+  expect(state.persistedState.searchInput).toEqual("c");
+  expect(state.persistedState.orderByKey).toEqual("d");
+  expect(state.persistedState.orderByAsc).toEqual(false);
+});