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

Get rid of recover from snapshot dialog #4328

Merged
merged 1 commit into from Mar 9, 2017
Merged

Conversation

voievodin
Copy link
Contributor

What does this PR do?

Removes recover from snapshot dialog from workspace start sequence.
The behaviour of IDE is now the same to dashboard, so recovery is enforced
by either of:

  • start workspace request query parameter restore
  • workspace attribute auto_restore
  • system configuration property che.workspace.auto_restore

The dialog view was:
image

It's still possible to choose whether to recover from snapshot or not after workspace stop:
image

Along with that i turned on snapshot status notifications:
image

image.

What issues does this PR fix or reference?

Resolves #3611

Changelog

Explicit snapshot creation becomes an internal approach.
Recover from snapshot dialog is removed from IDE.
Workspace REST API methods related to snapshots are deprecated.

Release Notes

API methods for creating and getting workspace snapshots are deprecated.
The reason is that snapshotting itself becomes an internal approach which is going to be
used on workspace stop only.

GET  /api/workspace/<id>/snapshot
POST /api/workspace/<id>/snapshot

Pull request #3405 introduced a deprecation of WorkspaceStatusEvent.getEventType() the alternative would be to use WorkspaceStatusEvent.getStatus(), but it turned out that usage of combination of status + previous status + optional error is not always convenient as the usage of the single event type, so it's reverted and not deprecated any more.

Docs PR

N/A

As a dialog alternative it could make sense to provide a kind of control like 'checkbox' on dashboard which would allow to decide per workspace whether to snapshot/recover it(discussion is welcome).

@voievodin voievodin self-assigned this Mar 6, 2017
@skabashnyuk skabashnyuk added this to the 5.5.0 milestone Mar 6, 2017
@voievodin voievodin requested a review from benoitf March 6, 2017 15:38
@mmorhun
Copy link
Contributor

mmorhun commented Mar 6, 2017

@evoevodin could you please explain how I can start a workspace ignoring its snapshot if auto restoring is turned on and I've closed tab after workspace stop?

@voievodin
Copy link
Contributor Author

As a dialog alternative it could make sense to provide a kind of control like 'checkbox' on dashboard which would allow to decide per workspace whether to snapshot/recover it(discussion is welcome).

From the API perspective you have 3 configuration options. From the UI no way, unless you go and edit workspace config directly on dashboard.

@bmicklea
Copy link

bmicklea commented Mar 6, 2017

@slemeur any opinions?

@slemeur
Copy link
Contributor

slemeur commented Mar 6, 2017

I see two different use cases:
1 - User should be able to configure the default behavior per workspace
2- User should be able to by-pass the default behavior

Configuring default behavior per workspace
In the workspace details: we can add a section to let the user being able to select how it should behave.

Allowing the user to by-pass the default behavior

  • In the left-sidebar: when doing a right click on the workspace, we could add an option to start the workspace with the different behaviors.
  • In the list of workspace: we could add the same options on the "cog" button
  • In the workspace details: we could add an arrow on the right of the "Open". When clicked it would show the different options.

@JamesDrummond
Copy link
Contributor

Deprecation requires notifying users prior to implementation. Do we plan to notify user in our release notes before merging?

I think we need to provide what @slemeur describes above before removing this option.

@bmicklea
Copy link

bmicklea commented Mar 6, 2017 via email

@codenvy-ci
Copy link

Build finished.
Build success. $BUILD_URL

@codenvy-ci
Copy link

@voievodin
Copy link
Contributor Author

voievodin commented Mar 7, 2017

I've added a workspace attribute snapshotted_at=1234567890 which is present only if workspace has a snapshot, it points to the time when the snapshot was created. This allows to implement the logic described by the #4337 without getting workspace snapshots from the client.

@bmicklea
Copy link

bmicklea commented Mar 7, 2017

@evoevodin: Stevan and I chatted - you have PM approval for merge.

@bmicklea bmicklea added the kind/enhancement A feature request - must adhere to the feature request template. label Mar 7, 2017
@codenvy-ci
Copy link

Build finished.
Build success. $BUILD_URL

@codenvy-ci
Copy link

@codenvy-ci
Copy link

Build finished.
Build success. $BUILD_URL

@codenvy-ci
Copy link

@codenvy-ci
Copy link

Build finished.
Build # 2138 - FAILED

Please check console output at $BUILD_URL to view the results.

@codenvy-ci
Copy link

Build # 2138 - FAILED

Please check console output at https://ci.codenvycorp.com/job/che-pullrequests-build/2138/ to view the results.

@voievodin
Copy link
Contributor Author

QA report on the changes

[TEST] Development state:
[TEST] Thu Mar 9 14:23:07 EET 2017. Failed tests (0):
[TEST]
[TEST] Comparing with https://ci.codenvycorp.com/view/qa/job/che-integration-tests/250/
[TEST] If a test failed then it is NOT marked as regression.
[TEST]
[TEST] NO REGRESSION, great job!

@voievodin voievodin merged commit 73c14ec into master Mar 9, 2017
@voievodin voievodin deleted the snapshot_explicit branch March 9, 2017 12:57
@codenvy-ci
Copy link

Build finished.
Build success. $BUILD_URL

@codenvy-ci
Copy link

@JamesDrummond JamesDrummond mentioned this pull request Mar 17, 2017
9 tasks
JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove explicit workspace snapshot creation from IDE
9 participants