Highlight experiments with errors #2072

mattseddon · 2022-07-20T09:38:40Z

This PR contains the first iteration of #1636.

Users will now be able to see experiment errors in the UI.

Demo

Screen.Recording.2022-07-21.at.1.06.00.pm.mov

Things to note:

We are not suppressing the selection of an experiment for plotting even if the experiment is completely broken.
We are currently not highlighting experiment errors in the plots webview. This will be covered under Handle DVC errors gracefully in plots #1649.
I updated the test fixture to contain error records which blew out the size of this PR (+~200 is for the test fixture by itself).
Styles match between the webview and the tree.
Errors cannot currently be filtered from the experiments table due to the hack we put in to stop queued items from being filtered by missing metrics.

mattseddon · 2022-07-21T01:29:47Z

extension/src/experiments/columns/extract.ts

+  const error = [
+    ...(metricsData?.errors || []),
+    ...(paramsData?.errors || [])
+  ].join('\n')


[I] Test this bit and the tooltip showing up in the UI as markdown.

mattseddon · 2022-07-21T01:54:10Z

extension/src/test/suite/experiments/model/filterBy/tree.test.ts

@@ -95,7 +95,7 @@ suite('Experiments Filter By Tree Test Suite', () => {
              )
            })
            .map(experiment =>
-              experiment.queued
+              experiment.queued || experiment.error


[F] This points out that the way we don't filter queued will spill over onto error records. You cannot filter by an undefined value which means that "full error" records will be "unfilterable".

mattseddon · 2022-07-21T01:54:57Z

extension/src/experiments/webview/contract.ts

@@ -16,19 +16,20 @@ export interface DepColumns {
 }

 export interface Experiment extends BaseExperimentFields {
+  deps?: DepColumns


[F] Sorted and added error?: string

mattseddon · 2022-07-21T01:56:18Z

extension/src/test/fixtures/expShow/noErrors.ts

+
+const data = excludeErrors()
+
+export default data


[F] Without this extra fixture there would be a lot of changes to plots tests which are currently unrelated to this change.

And this is why the likes of extension/src/test/suite/plots/index.test.ts are now pointed at this.

When I come back to do plots errors I should be able to remove this. As we'll want to highlight the behaviour that we can have experiment errors but still try to plot those experiments.

mattseddon · 2022-07-21T01:59:36Z

webview/src/experiments/components/table/Errors.tsx

@@ -0,0 +1,22 @@
+import React from 'react'


[Q] Better placement for this? Should it be a shared component? We will probably use something very similar for plots.

We can move it in the shared folder when it's used in both webviews.

…xp-errors

mattseddon · 2022-07-21T03:16:11Z

extension/src/experiments/index.ts

@@ -16,7 +16,7 @@ import { ExperimentsData } from './data'
 import { askToDisableAutoApplyFilters } from './toast'
 import { Experiment, ColumnType, TableData } from './webview/contract'
 import { WebviewMessages } from './webview/messages'
-import { DecorationProvider } from './model/filterBy/decorationProvider'
+import { DecorationProvider } from './model/decorationProvider'


[F] Move to a more central location as the decorations are now for both errors and filtered experiments

mattseddon · 2022-07-21T03:17:12Z

extension/src/experiments/model/decorationProvider.ts

@@ -38,19 +43,27 @@ export class DecorationProvider
  }

  public provideFileDecoration(uri: Uri): FileDecoration | undefined {
+    if (this.errors.has(uri.fsPath)) {
+      return DecorationProvider.DecorationError


[F] Take priority but under the current implementation, we can't have both.

mattseddon · 2022-07-21T03:18:37Z

extension/src/test/fixtures/expShow/output.ts

+    [errorShas[0]]: {
+      error: {
+        type: 'YAMLFileCorruptedError',
+        msg: "unable to read: 'params.yaml', YAML file structure is corrupted"


[F] The fixture now contains one "full error" and one "partial error".

maxagin · 2022-07-21T04:15:27Z

Styles match between the webview and the tree.

@mattseddon this is only for now? Right? Will we have the styles for the table as we planned?

mattseddon · 2022-07-21T04:58:11Z

Styles match between the webview and the tree.

@mattseddon this is only for now? Right? Will we have the styles for the table as we planned?

I have implemented everything as per my latest comments in the spec. The main point here is to highlight to users that there is an error and give them information about the error. We can adjust later when we get plot errors or user feedback.

sroy3 · 2022-07-21T13:39:29Z

webview/src/experiments/components/table/Errors.tsx

@@ -0,0 +1,22 @@
+import React from 'react'


We can move it in the shared folder when it's used in both webviews.

sroy3 · 2022-07-21T13:45:51Z

webview/src/shared/components/icons/Error.tsx

@@ -0,0 +1,21 @@
+import * as React from 'react'


We decided to remove theAllIcons enum that gave us the ability to view all available icons since it was duplicating the index file in the icons folder. We should either export this inside the index file or simply remove the index file.

I think we have a couple of other direct imports. I'll track them down today and move all imports to come from the index file. Let me know if you'd like to go the other way.

codeclimate · 2022-07-21T20:08:36Z

Code Climate has analyzed commit 6008904 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 94.8% (85% is the threshold).

This pull request will bring the total coverage in the repository to 96.7% (0.0% change).

View more on Code Climate.

mattseddon added the product PR that affects product label Jul 20, 2022

mattseddon self-assigned this Jul 20, 2022

mattseddon changed the title ~~Show experiment errors~~ Highlight experiments with errors Jul 20, 2022

mattseddon added 4 commits July 20, 2022 19:42

implement basic errors functionality

d873169

add partial errors

04cbe8f

refactor errors in experiments webview

30efa06

pull out type

2b2076a

mattseddon force-pushed the display-exp-errors branch from f2cdb71 to 2b2076a Compare July 20, 2022 09:42

mattseddon added 2 commits July 20, 2022 20:01

fix test

7a50b8a

use pre-wrap for tooltip (inserts line breaks)

ce9a0c2

mattseddon commented Jul 21, 2022

View reviewed changes

use cell contents component

139eabc

mattseddon commented Jul 21, 2022

View reviewed changes

mattseddon added 4 commits July 21, 2022 12:35

add tooltip test in tree

f4a6852

add test for error handling in extract columns

4a4735b

Merge branch 'main' of github.com:iterative/vscode-dvc into display-e…

7620541

…xp-errors

update tests after merging main

0077849

mattseddon commented Jul 21, 2022

View reviewed changes

mattseddon marked this pull request as ready for review July 21, 2022 03:21

mattseddon requested review from rogermparent, sroy3, wolmir and julieg18 as code owners July 21, 2022 03:21

mattseddon requested review from maxagin and shcheklein July 21, 2022 03:21

sroy3 approved these changes Jul 21, 2022

View reviewed changes

julieg18 approved these changes Jul 21, 2022

View reviewed changes

Merge branch 'main' into display-exp-errors

6008904

mattseddon merged commit 743f826 into main Jul 21, 2022

mattseddon deleted the display-exp-errors branch July 21, 2022 21:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Highlight experiments with errors #2072

Highlight experiments with errors #2072

mattseddon commented Jul 20, 2022 •

edited

Loading

mattseddon Jul 21, 2022

mattseddon Jul 21, 2022

mattseddon Jul 21, 2022

mattseddon Jul 21, 2022

mattseddon Jul 21, 2022

mattseddon Jul 21, 2022

mattseddon Jul 21, 2022

sroy3 Jul 21, 2022

mattseddon Jul 21, 2022

mattseddon Jul 21, 2022

mattseddon Jul 21, 2022

maxagin commented Jul 21, 2022

mattseddon commented Jul 21, 2022

sroy3 Jul 21, 2022

sroy3 Jul 21, 2022

mattseddon Jul 21, 2022

codeclimate bot commented Jul 21, 2022

Highlight experiments with errors #2072

Highlight experiments with errors #2072

Conversation

mattseddon commented Jul 20, 2022 • edited Loading

Demo

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxagin commented Jul 21, 2022

mattseddon commented Jul 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codeclimate bot commented Jul 21, 2022

mattseddon commented Jul 20, 2022 •

edited

Loading