Remove wkw datasource suggestions #7697

frcroth · 2024-03-18T15:20:46Z

Removes the automatic datasource-properties.json suggestions for the Dataset Settings view. They only worked for WKW and confusing errors popped up for other data formats. Also, since most dataset creation code already generates valid jsons, it was mostly unused.

At some point in the future we may want to use the explore code to bring them back, if that turns out to be a comon use case. (We’d probably have to add an Explorer for WKW).

Steps to test:

TODOs:

Backend
- Remove exploring route
- remove unused code
- Adapt readInboxDatasource route to support reporting the most current version from disk
Frontend
- Use readInboxDataSource in dataset settings view (note that it may contain existingDataSourceProperties in case of Unusable json)
- Decide what to show exactly in case of invalid json (both invalid fields and json parse error) -> an error :)
- In dashboard, always call the link “Settings”, no longer “Import”
- What should the frontend to in case the datasource-properties.json is not valid? / cannot be parsed?

Issues:

fixes Unify exploring zarr and importing wkw #7474

Updated changelog
Updated migration guide if applicable
Updated documentation if applicable
Removed dev-only changes like prints and application.conf edits
Considered common edge cases
Needs datastore update after deployment

- but keep isEditMode prop due to onboarding process needing this prop

…ove-wkw-suggestions

MichaelBuessemeyer · 2024-05-24T15:23:27Z

This is how the settings view looks like, in case the datasource-properties.json is not a valid json file:

And like this, in case the "name" property is missing in the "id" field

I'd say it is okaisch, or do we want to improve the suggestion?

philippotto · 2024-05-27T07:56:22Z

I'd say it is okaisch

Yes, since manually edited jsons are rare, it's okay in my opinion.

MichaelBuessemeyer · 2024-05-24T14:57:35Z

frontend/javascripts/admin/dataset/dataset_add_view.tsx

                <Button
-                  onClick={() => history.push(`/datasets/${organization}/${datasetName}/import`)}
+                  onClick={() => history.push(`/datasets/${organization}/${datasetName}/edit`)}


As according to the pr description, the "import" should always be named edit, I got rid of the import route (see router.tsx). I hope I didn't misunderstand this 🤞

MichaelBuessemeyer · 2024-05-24T15:03:51Z

frontend/javascripts/dashboard/advanced_dataset/dataset_action_view.tsx

@@ -137,54 +131,6 @@ function DatasetActionView(props: Props) {
    setIsReloading(false);
  };

-  const onDeleteDataset = async () => {


This function was only used in the importLink. And as I removed this, I also removed this function here

MichaelBuessemeyer · 2024-05-28T09:14:34Z

frontend/javascripts/dashboard/advanced_dataset/dataset_action_view.tsx

@@ -330,7 +236,7 @@ export function getDatasetActionContextMenu({
            },
          }
        : null,
-      dataset.isEditable && dataset.isActive


Am I correct, that we no longer have a "not imported" state for a dataset? In that case, is it possible for a dataset to not be active? The only option would be to delete a dataset 🤔

My thought is that we may not need this field isActive anymore / do not need to consider it any longer in the frontend 🤔

Am I correct, that we no longer have a "not imported" state for a dataset? In that case, is it possible for a dataset to not be active?

I don't know what happens now if a datasource properties json does not exist (or is incomplete). I assume the dataset won't be active then. However, the front-end should still show the edit button. Otherwise, the dataset would not be usable/fixable as you say.

MichaelBuessemeyer · 2024-05-28T09:17:40Z

frontend/javascripts/dashboard/dataset/dataset_settings_view.tsx

      // Ensure that zarr layers (which aren't inferred by the back-end) are still
      // included in the inferred data source
-      if (
-        savedDataSourceOnServer &&
-        "dataLayers" in savedDataSourceOnServer &&
-        inferredDataSource != null
-      ) {


This cannot be done as there is no longer a inferredDataSource. So does this maybe result in errors with zarr layers?

MichaelBuessemeyer · 2024-05-28T12:18:37Z

I'd say the frontend changes are ready for the first review / comments

philippotto · 2024-05-28T15:24:29Z

frontend/javascripts/dashboard/advanced_dataset/dataset_action_view.tsx

-  const importLink = (
-    <div className="dataset-table-actions">
-      <Link
-        to={`/datasets/${dataset.owningOrganization}/${dataset.name}/import`}
-        className="import-dataset"
-      >
-        <PlusCircleOutlined className="icon-margin-right" />
-        Import
-      </Link>
-      {reloadLink}
-      <a
-        onClick={() =>
-          Modal.error({
-            title: "Cannot load this dataset",
-            content: (
-              <div>
-                <p>{dataset.status}</p>
-                {dataset.status === "Deleted by user." ? (
-                  <p>
-                    Even though this dataset was deleted by a user, it is still shown here, because
-                    it was referenced by at least one annotation.
-                  </p>
-                ) : null}
-              </div>
-            ),
-          })
-        }
-      >
-        <WarningOutlined className="icon-margin-right" />
-        Show Error
-      </a>
-      {dataset.status !== "Deleted by user." ? (
-        <a onClick={() => onDeleteDataset()}>
-          <DeleteOutlined className="icon-margin-right" />
-          Delete Dataset
-        </a>


I'm not sure whether it's a good idea to delete this. The use case is:

a dataset exists on disk, but wk isn't able to read it properly (e.g., because of an invalid json)

in that case, as a user I want to be able to see the issue, be able to fix it or to delete the dataset directly from the table.

I don't think that removing the wkw datasource suggestions means that the above use cases goes away.

cc @fm3

I agree, it should still be possible to fix the json, and parsing errors should be reported. Just not the automated suggestions.

philippotto · 2024-05-28T15:26:23Z

frontend/javascripts/dashboard/dataset/dataset_settings_view.tsx

-        messages: dataSourceMessages,
-        dataSourceSettingsStatus,
-      });
+      // TODOM: handle case in which dataSource is null / could not be parsed by the server


please see my post below :)
I'd say both cases are handle relatively adequately. Thus, I removed this todo

MichaelBuessemeyer · 2024-05-29T09:23:18Z

This is how it looks if there is no datasource-properties.json available in the backend in the dataset list view on the newest version of the branch:

And this in the settings view:

In case the datasource-properties.json contains errors the dataset table looks the same as above.
And the settings look like this:

I'm not sure whether it's a good idea to delete this. The use case is:
a dataset exists on disk, but wk isn't able to read it properly (e.g., because of an invalid json)

This should be handled now: The dataset table always gives the option to a user to edit the settings (as long as the user has the capabilities (isEditable)).

in that case, as a user I want to be able to see the issue, be able to fix it or to delete the dataset directly from the table.

The settings view gives a warning about not being able to parse the datasource-properties.json (although the warning is not easy to understand). And the settings are accessible from the dataset table.
I do not understand why one would want to delete a dataset directly from the dataset table view. This is currently not possible but I think I also misunderstand your part

to delete the dataset directly from the table.

here a little.

philippotto · 2024-05-29T09:37:27Z

I do not understand why one would want to delete a dataset directly from the dataset table view. This is currently not possible but I think I also misunderstand your part [...] here a little.

This feature was added not long ago. See here for context. I think, it would be good to keep it :)

…ove-wkw-suggestions

…ets in dataset table

MichaelBuessemeyer · 2024-06-04T15:38:33Z

I'd say I applied all requested cases. How to we want to proceed with this @fm3?

Maybe we can check together my changes requested by philipp and then merge this once its clear to go?

fm3 · 2024-06-05T09:53:27Z

Do you think that letting this PR lie will create many merge conflicts? If not, I’d say it’s fair to wait until Philipp returns.

philippotto

sweet, red lines :) front-end lgtm 👍

…knossos into remove-wkw-suggestions

frcroth added 2 commits March 18, 2024 16:20

Remove suggestion route

2849111

Remove WKWDataFormat

c9f752a

frcroth force-pushed the remove-wkw-suggestions branch from 77d5fce to c9f752a Compare March 18, 2024 15:42

Lint

de8017a

frcroth added the frontend label Mar 18, 2024

fm3 self-assigned this Mar 21, 2024

fm3 added 4 commits March 21, 2024 16:44

Merge branch 'master' into remove-wkw-suggestions

c1b52f4

wip remove in frontend

b264843

Merge branch 'master' into remove-wkw-suggestions

1673c15

unify readInboxDataSource

c1c4630

fm3 added backend refactoring labels Apr 2, 2024

philippotto requested review from philippotto and removed request for philippotto May 6, 2024 09:21

philippotto assigned MichaelBuessemeyer May 6, 2024

MichaelBuessemeyer added 3 commits May 24, 2024 15:46

remove datasource suggestion

97a9d6d

- but keep isEditMode prop due to onboarding process needing this prop

Merge branch 'master' of github.com:scalableminds/webknossos into rem…

1d5437b

…ove-wkw-suggestions

remove left over code line linking to dataset import and clean up code

b0be7cd

remove unused imports in backend

5dd5e4d

MichaelBuessemeyer reviewed May 27, 2024

View reviewed changes

MichaelBuessemeyer reviewed May 28, 2024

View reviewed changes

MichaelBuessemeyer requested a review from philippotto May 28, 2024 12:18

philippotto reviewed May 28, 2024

View reviewed changes

always show edit settings for datasets the user can edit

cb4d67f

MichaelBuessemeyer marked this pull request as ready for review May 29, 2024 09:38

MichaelBuessemeyer added 3 commits May 31, 2024 14:16

Merge branch 'master' of github.com:scalableminds/webknossos into rem…

ef4188b

…ove-wkw-suggestions

remove unused type

db9ba6d

er-add dataset actions (show error & delete dataset) for broken datas…

f755c22

…ets in dataset table

MichaelBuessemeyer requested a review from philippotto May 31, 2024 12:56

Merge branch 'master' into remove-wkw-suggestions

724445d

philippotto approved these changes Jun 17, 2024

View reviewed changes

fm3 and others added 6 commits June 18, 2024 08:49

Merge branch 'master' into remove-wkw-suggestions

5f7c546

adapt wording in messages

b5fb853

add changelog entry

67e3295

Merge branch 'remove-wkw-suggestions' of github.com:scalableminds/web…

dd1f4e1

…knossos into remove-wkw-suggestions

update migrationguide,changelog,docs

c1cee08

Merge branch 'remove-wkw-suggestions' of github.com:scalableminds/web…

dab6a0c

…knossos into remove-wkw-suggestions

fm3 enabled auto-merge (squash) June 18, 2024 07:51

fm3 merged commit 5bf95c8 into master Jun 18, 2024
2 checks passed

fm3 deleted the remove-wkw-suggestions branch June 18, 2024 07:53

MichaelBuessemeyer mentioned this pull request Jun 26, 2024

Fix not being able to save dataset settings #7903

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove wkw datasource suggestions #7697

Remove wkw datasource suggestions #7697

frcroth commented Mar 18, 2024 •

edited by fm3

Loading

MichaelBuessemeyer commented May 24, 2024

philippotto commented May 27, 2024

MichaelBuessemeyer May 24, 2024

MichaelBuessemeyer May 24, 2024

MichaelBuessemeyer May 28, 2024

philippotto May 28, 2024

MichaelBuessemeyer May 28, 2024

MichaelBuessemeyer commented May 28, 2024

philippotto May 28, 2024

fm3 May 30, 2024

philippotto May 28, 2024

MichaelBuessemeyer May 29, 2024 •

edited

Loading

MichaelBuessemeyer commented May 29, 2024

philippotto commented May 29, 2024

MichaelBuessemeyer commented Jun 4, 2024

fm3 commented Jun 5, 2024

philippotto left a comment

Remove wkw datasource suggestions #7697

Remove wkw datasource suggestions #7697

Conversation

frcroth commented Mar 18, 2024 • edited by fm3 Loading

Steps to test:

TODOs:

Issues:

MichaelBuessemeyer commented May 24, 2024

philippotto commented May 27, 2024

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

MichaelBuessemeyer commented May 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MichaelBuessemeyer May 29, 2024 • edited Loading

Choose a reason for hiding this comment

MichaelBuessemeyer commented May 29, 2024

philippotto commented May 29, 2024

MichaelBuessemeyer commented Jun 4, 2024

fm3 commented Jun 5, 2024

philippotto left a comment

Choose a reason for hiding this comment

frcroth commented Mar 18, 2024 •

edited by fm3

Loading

MichaelBuessemeyer May 29, 2024 •

edited

Loading