Add --latest flag to restore-snapshot command, fixes #1886, fixes #1163 #2724

cmuench · 2021-01-02T13:25:29Z

The Problem/Issue/Bug:

A option to restore the latest snapshot is missing. See #1886 and #1163
Currently we need to pass the name of the latest snapshot as argument.

How this PR Solves The Problem:

Add new --latest flag to restore-snapshot command.
Modify the sort of the app.ListSnapshots method. Sort files by last modification date.
A new method app.GetLatestSnapshot is introduced.

Manual Testing Instructions:

Automated Testing Overview:

Test for command and app GetLatestSnapshot is included.

Release/Deployment notes:

Changes are backwards compatible.

rfay · 2021-01-02T13:46:55Z

What do you think about adding --list to this one while you're here as well? The biggest problem with restore-snapshot is figuring out which one, and that would help loads. Autocomplete would help even more, but I don't have a clue how to add non-static autocompletes.

cmuench · 2021-01-02T15:03:52Z

@rfay It depends on the concept. We could also add a --restore flag to the snapshot command.

About interaction...
I have a lot experience in building cli application with interaction. I am the maintainer of n98-magerun which is the Magento CLI tool (it's PHP with Symfony Console Component). If we can add readline support it should be possible to have more interaction there. It would be great to select the snapshot if no name is passed. I hope there is a good CLI library in golang for that.

cmuench · 2021-01-02T15:07:32Z

@rfay Found that: https://github.com/manifoldco/promptui

rfay · 2021-01-02T15:14:02Z

I really think "restore" should have been a subcommand of snapshot in the first place. ddev snaspshot restore [flags]

I guess it would be a fairly easy thing to add that and then remove the restore-snapshot in a year or so.

cmuench · 2021-01-02T15:17:23Z

I really think "restore" should have been a subcommand of snapshot in the first place. ddev snaspshot restore [flags]

ok. Let me try that out. I can also try out the promptui there.

cmuench · 2021-01-03T14:18:35Z

@rfay I added the sub-command and also the interaction.

The interaction looks really nice:

Currently the tests are broken. I do not understand where the problem in the build is. Maybe you can have a look on it to give me a hint.

rfay · 2021-01-03T15:06:45Z

There were a couple of painful windows test runner failures, but it looks like right now TestGetLatestSnapshot just doesn't work. That should show up with a manual test run in GoLand or running that single test as in https://github.com/drud/ddev/blob/master/docs/developers/building-contributing.md#testing

cmuench · 2021-01-03T15:14:26Z

There were a couple of painful windows test runner failures, but it looks like right now TestGetLatestSnapshot just doesn't work. That should show up with a manual test run in GoLand or running that single test as in https://github.com/drud/ddev/blob/master/docs/developers/building-contributing.md#testing

Thanks. Will try it.

rfay · 2021-01-03T16:25:48Z

Wow, this terminal library works on Windows in git-bash, PS, and cmd! I've never found one that would do that. We could do so very many things with that!

One nit - restore-snapshot shows up in the list twice when you just type "ddev"

I've reconsidered what I said yesterday, and think we can just move ddev restore-snapshot completely to ddev snapshot restore and leave a hidden restore-snapshot command that just says "please use ddev snapshot restore"

rfay · 2021-01-03T16:34:00Z

It's truly lovely.

I don't think the "restore" is properly in there as a subcommand? Because I can't get help for it, ddev snapshot restore -h doesn't get something useful. ddev debug might be a pattern to use.

cmuench · 2021-01-03T19:40:34Z

@rfay Everything is committed. Also the new sub-command. Tests are in general "green". There is a download/caching issue in the buildkite/ddev-windows-apache-fpm build job. IMHO it's not related to the changes of the PR.
The previoud failing tests were related to data generated during the test run. I was able to modify the tests so that they are more stable, now.

rfay · 2021-01-03T22:07:25Z

Just gave it a quick spin. Looks absolutely fantastic!

rfay · 2021-01-03T22:38:56Z

@cmuench do you have CircleCI building on your own account? I see that it's not reporting here.

cmuench · 2021-01-03T22:44:09Z

@cmuench do you have CircleCI building on your own account? I see that it's not reporting here.

Did not configure it.

rfay · 2021-01-03T22:45:39Z

Weird, I'll chase it. But we sure want that full test. I think what you're doing here would be sorted out by the tests that are running, but the CircleCI tests are the biggest batch.

rfay

Absolutely fantastic, and opens the door for more prompted choices. ddev config <provider> once had a version that did exactly like this, but there was no way to make it work on Windows and no way to test it.

Made a number of minor requests, the ones with suggestions you can batch and add.

One minor nit,

cmd/ddev/cmd/restore_snapshot.go

cmd/ddev/cmd/snapshot_restore.go

rfay · 2021-01-04T16:46:01Z

cmd/ddev/cmd/snapshot_restore.go

+
+				if len(snapshots) == 0 {
+					util.Failed("No snapshots found for project %s", app.GetName())
+					os.Exit(1)


Suggested change

os.Exit(1)

cmd/ddev/cmd/snapshot_restore.go

cmd/ddev/cmd/snapshot_restore_test.go

docs/users/cli-usage.md

cmd/ddev/cmd/snapshot_test.go

rfay · 2021-01-04T18:36:30Z

I'm sure you saw the email from CircleCI - they suggest you might have clicked "follow" on the CircleCI page and so now you have to unfollow. Weirder than weird.

The situation you reported is most likely due to the fact that the user who created the pull-requests is also following the project fork on their personal CircleCI account (https://support.circleci.com/hc/en-us/articles/360008097173-Why-aren-t-pull-requests-triggering-jobs-on-my-organization-)

I would have thought only building on your fork would cause this.

cmuench · 2021-01-05T08:51:06Z

I'm sure you saw the email from CircleCI - they suggest you might have clicked "follow" on the CircleCI page and so now you have to unfollow. Weirder than weird.

The situation you reported is most likely due to the fact that the user who created the pull-requests is also following the project fork on their personal CircleCI account (https://support.circleci.com/hc/en-us/articles/360008097173-Why-aren-t-pull-requests-triggering-jobs-on-my-organization-)

I would have thought only building on your fork would cause this.

I am not following the project. Did not know that there is a circle-ci integration, before you mention it.
BTW I get this message if I went to the circle-ci page:

"There are no streaming updates available on this page because you do not follow this project."

rfay · 2021-01-05T11:55:31Z

Thanks for responding to the CircleCI folks. And I see it's being triggered now. No idea what was going on, but thanks for all!

rfay · 2021-01-09T04:36:16Z

Rebased.

rfay

Looks great, fantastic!

rfay · 2021-01-10T16:15:57Z

@cmuench if you're not exhausted with snapshots there are two remaining open issues regarding snapshots, and your brain is in there. They seem far simpler than the things you've already undertaken. But I know you don't have unlimited time, and also may be done with snapshots :) (Edit: I did the don't-reuse-existing-name one in #2739)

rfay · 2021-01-10T17:10:31Z

(Edit: I did the don't-reuse-existing-name one in #2739)

chellman mentioned this pull request Jan 4, 2021

Improve ddev snapshot UI - Suggest or require snapshot name, etc. #1886

Closed

rfay requested changes Jan 4, 2021

View reviewed changes

rfay changed the title ~~Add --latest flag to restore-snapshot command~~ Add --latest flag to restore-snapshot command, fixes #1886 Jan 5, 2021

rfay linked an issue Jan 5, 2021 that may be closed by this pull request

Improve ddev snapshot UI - Suggest or require snapshot name, etc. #1886

Closed

rfay changed the title ~~Add --latest flag to restore-snapshot command, fixes #1886~~ Add --latest flag to restore-snapshot command, fixes #1886, fixes #1163 Jan 5, 2021

cmuench requested a review from rfay January 7, 2021 21:25

cmuench added 6 commits January 8, 2021 21:30

Add --latest flag to restore-snapshot command

68c4eb6

Move new snapshot restore logic into "snapshot restore" sub-command

be4ef7c

Add promptui dependency

9d31bc8

Change snapshot restore documentation

e364001

Add interactive snapshot selection

8516bc8

Fix error messages

c99b8ca

cmuench added 9 commits January 8, 2021 21:30

Polish tests and change assert for build pipeline

0379399

Hide old restore-snapshot command

7b9a4f6

Cleanup old existing snapshots before we create one

1b02025

Change interactive prompt/label and remove exit calls

8aa4858

Describe new snapshot restore method

75c26d9

Remove unnecessary check if project omit db container

10d9cce

Add test for interactive snapshot restore

8369d23

Remove unnecessary start of project in test cleanup

8dfb852

Start project before snapshot test

5acf08f

rfay force-pushed the feature/snapshot-restore-latest branch from 40e77d9 to 5acf08f Compare January 9, 2021 04:31

rfay approved these changes Jan 10, 2021

View reviewed changes

rfay merged commit 51ede53 into ddev:master Jan 10, 2021

cmuench deleted the feature/snapshot-restore-latest branch January 10, 2021 17:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add --latest flag to restore-snapshot command, fixes #1886, fixes #1163 #2724

Add --latest flag to restore-snapshot command, fixes #1886, fixes #1163 #2724

cmuench commented Jan 2, 2021 •

edited by rfay

rfay commented Jan 2, 2021

cmuench commented Jan 2, 2021

cmuench commented Jan 2, 2021

rfay commented Jan 2, 2021

cmuench commented Jan 2, 2021

cmuench commented Jan 3, 2021

rfay commented Jan 3, 2021

cmuench commented Jan 3, 2021

rfay commented Jan 3, 2021

rfay commented Jan 3, 2021

cmuench commented Jan 3, 2021 •

edited

rfay commented Jan 3, 2021

rfay commented Jan 3, 2021

cmuench commented Jan 3, 2021

rfay commented Jan 3, 2021

rfay left a comment

rfay Jan 4, 2021

rfay commented Jan 4, 2021

cmuench commented Jan 5, 2021

rfay commented Jan 5, 2021

rfay commented Jan 9, 2021

rfay left a comment

rfay commented Jan 10, 2021 •

edited

rfay commented Jan 10, 2021

Add --latest flag to restore-snapshot command, fixes #1886, fixes #1163 #2724

Add --latest flag to restore-snapshot command, fixes #1886, fixes #1163 #2724

Conversation

cmuench commented Jan 2, 2021 • edited by rfay

The Problem/Issue/Bug:

How this PR Solves The Problem:

Manual Testing Instructions:

Automated Testing Overview:

Release/Deployment notes:

rfay commented Jan 2, 2021

cmuench commented Jan 2, 2021

cmuench commented Jan 2, 2021

rfay commented Jan 2, 2021

cmuench commented Jan 2, 2021

cmuench commented Jan 3, 2021

rfay commented Jan 3, 2021

cmuench commented Jan 3, 2021

rfay commented Jan 3, 2021

rfay commented Jan 3, 2021

cmuench commented Jan 3, 2021 • edited

rfay commented Jan 3, 2021

rfay commented Jan 3, 2021

cmuench commented Jan 3, 2021

rfay commented Jan 3, 2021

rfay left a comment

Choose a reason for hiding this comment

rfay Jan 4, 2021

Choose a reason for hiding this comment

rfay commented Jan 4, 2021

cmuench commented Jan 5, 2021

rfay commented Jan 5, 2021

rfay commented Jan 9, 2021

rfay left a comment

Choose a reason for hiding this comment

rfay commented Jan 10, 2021 • edited

rfay commented Jan 10, 2021

cmuench commented Jan 2, 2021 •

edited by rfay

cmuench commented Jan 3, 2021 •

edited

rfay commented Jan 10, 2021 •

edited