MAGECLOUD-1372: Backup Critical Configuration Files to Recover On-Demand #115

BaDos · 2017-12-08T08:57:40Z

Description

After each redeploy will be created backup of configuration files: app/etc/config.php.bak app/etc/env.php.bak

If user removed or broke these files he can restore them using the next command:
./vendor/bin/ece-tools backup:restore

Also user can see available backup files using ./vendor/bin/ece-tools backup:list

Fixed Issues (if relevant)

https://magento2.atlassian.net/browse/MAGECLOUD-1372

Zephyr Tests

https://jira.corp.magento.com/browse/MAGETWO-85289

Contribution checklist

Pull request has a meaningful description of its purpose
Pull request was approved by architect
Pull request was approved by QA member
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

oshmyheliuk · 2017-12-08T09:16:39Z

src/Command/BackupList.php

+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+


please remove empty line

Fixed. Tanks.

oshmyheliuk · 2017-12-08T09:19:22Z

src/Command/BackupList.php

+     */
+    protected function execute(InputInterface $input, OutputInterface $output)
+    {
+


please remove line

oshmyheliuk · 2017-12-08T09:22:13Z

src/Command/BackupList.php

+
+        try {
+            $output->writeln('<comment>The list of backup files:</comment>');
+            $output->writeln(array_keys($this->backupFilesList->getList()));


Maybe you should check if file is backup file really exists?

shiftedreality · 2017-12-08T09:22:34Z

src/Backup/Restorer.php

+ *
+ * @see \Magento\MagentoCloud\Filesystem\BackupList contains the list of files for restoring
+ */
+class Restorer


Rename to Restore

shiftedreality · 2017-12-08T09:26:21Z

src/Backup/Restorer.php

+        $backupList = $this->backupList->getList();
+
+        if ($specificPath) {
+            if (empty($backupList[$specificPath])) {


Move this check to restore() method

No, this we need this check only if a user wants to restore specific file.

shiftedreality · 2017-12-08T09:28:51Z

src/Command/BackupRestore.php

+/**
+ * CLI command for restoring Magento configuration files from backup.
+ */
+class BackupRestore extends Command


Move this to ../Command/Backup/Restore namespace

NadiyaS · 2017-12-08T09:36:49Z

src/Backup/Restorer.php

+     *
+     * @param InputInterface $input
+     * @param OutputInterface $output
+     * @param string $alias


Describe what does alias mean

and better to change the order of arguments. I see that $alias may be not required when you cannot implement method without $file (file name)

NadiyaS · 2017-12-08T09:37:07Z

src/Backup/Restorer.php

+     * @param OutputInterface $output
+     * @param string $alias
+     * @param string $file
+     */


add @return void

There is @return in my locale code :)

NadiyaS · 2017-12-08T09:47:35Z

src/Command/BackupList.php

+        } catch (\Exception $exception) {
+            $this->logger->critical($exception->getMessage());
+
+            throw $exception;


in the declaration this method should return only LogicException and only when this abstract method is not implemented

So... we need to fix all our CLI commands...

NadiyaS · 2017-12-08T09:51:23Z

src/Command/BackupRestore.php

+    protected function configure()
+    {
+        $this->setName(self::NAME)
+            ->setDescription('Restore important configuration files');


Where may the user take info about which exactly important file will be restored?
As I see this command may be run without name of file, so all files will be restored, so it is important to make user know about what he is doing.
At least you need to forward the user to the command which can provide this information

NadiyaS · 2017-12-08T09:57:38Z

src/Command/BackupList.php

+    /**
+     * Command name
+     */
+    const NAME = 'backup:list';


I see backup:list and backup:restore. Where is the command which creates these backups?

We have a process that creates backups after deploy.

NadiyaS · 2017-12-11T08:59:50Z

src/App/Container.php

@@ -153,6 +153,10 @@ function () use ($root, $config) {
                        $this->container->make(DeployProcess\CompressStaticContent::class),
                        $this->container->make(DeployProcess\DisableGoogleAnalytics::class),
                        $this->container->make(DeployProcess\UnlockCronJobs::class),
+                        /**
+                         * Remove this line after implementation post-deploy hook


then maybe add todo. And add this point to the ticket with post-deploy or even additional task is better

Yes, I will create additional task

https://magento2.atlassian.net/browse/MAGECLOUD-1412

oshmyheliuk · 2017-12-11T09:00:46Z

src/Command/BackupList.php

+        try {
+            $output->writeln('<comment>The list of backup files:</comment>');
+            $output->writeln($this->backupFilesList->get() ?: 'There are no files in the backup');
+            return 0;


Please use constants, or don't return anything.

oshmyheliuk · 2017-12-11T09:06:47Z

src/Command/BackupList.php

+            return 0;
+        } catch (\Exception $exception) {
+            $this->logger->critical($exception->getMessage());
+            return 1;


Please use constant or throw an exception.

oshmyheliuk · 2017-12-11T09:39:00Z

src/Command/BackupRestore.php

+                . ' with option --force rewrite your existed files. Do you want to continue [y/N]?',
+                false
+            );
+            $restore = $helper->ask($input, $output, $question);


Please move this into try-catch, ask can throw an exception.

Agree, fixed

oshmyheliuk · 2017-12-11T09:39:39Z

src/Command/BackupRestore.php

+            if ($restore) {
+                $this->restore->run($input, $output);
+            }
+            return 0;


Replace with constant.

NadiyaS · 2017-12-11T11:27:34Z

src/Command/BackupRestore.php

+            $helper = $this->getHelper('question');
+            $question = new ConfirmationQuestion(
+                'Command ' . self::NAME
+                . ' with option --force rewrite your existed files. Do you want to continue [y/N]?',


change the text

... will rewrite ...

BaDos · 2017-12-11T11:34:49Z

https://magento2.atlassian.net/browse/MAGECLOUD-1411

andriyShevtsov · 2017-12-11T15:05:02Z

QA approved

) MCLOUD-10250: update broken onboarding docs link in email template

Bohdan Korablov added 6 commits December 7, 2017 15:16

MAGECLOUD-1372: Backup Critical Configuration Files to Recover On-Demand

86696f4

MAGECLOUD-1372: Backup Critical Configuration Files to Recover On-Demand

eab2bb3

MAGECLOUD-1372: Backup Critical Configuration Files to Recover On-Demand

31fb4ed

MAGECLOUD-1372: Backup Critical Configuration Files to Recover On-Demand

65953ea

MAGECLOUD-1372: Backup Critical Configuration Files to Recover On-Demand

95d7ea5

MAGECLOUD-1372: Backup Critical Configuration Files to Recover On-Demand

1bccb24

oshmyheliuk reviewed Dec 8, 2017

View reviewed changes

shiftedreality suggested changes Dec 8, 2017

View reviewed changes

shiftedreality added the Progress: review PR/Issue status label Dec 8, 2017

shiftedreality assigned oshmyheliuk Dec 8, 2017

NadiyaS reviewed Dec 8, 2017

View reviewed changes

Bohdan Korablov added 2 commits December 8, 2017 15:59

MAGECLOUD-1372: Backup Critical Configuration Files to Recover On-Demand

59f8289

MAGECLOUD-1372: Backup Critical Configuration Files to Recover On-Demand

3f4eb48

NadiyaS reviewed Dec 11, 2017

View reviewed changes

oshmyheliuk reviewed Dec 11, 2017

View reviewed changes

NadiyaS reviewed Dec 11, 2017

View reviewed changes

Bohdan Korablov added 2 commits December 11, 2017 13:27

MAGECLOUD-1372: Backup Critical Configuration Files to Recover On-Demand

c85a62a

MAGECLOUD-1372: Backup Critical Configuration Files to Recover On-Demand

82ed77b

NadiyaS approved these changes Dec 11, 2017

View reviewed changes

shiftedreality added Progress: testing in progress PR/issue status and removed Progress: review PR/Issue status labels Dec 11, 2017

shiftedreality assigned andriyShevtsov and unassigned oshmyheliuk Dec 11, 2017

shiftedreality approved these changes Dec 11, 2017

View reviewed changes

oshmyheliuk approved these changes Dec 11, 2017

View reviewed changes

BaDos added Progress: accept PR/issue status and removed Progress: testing in progress PR/issue status labels Dec 11, 2017

shiftedreality merged commit 6fc653a into magento:develop Dec 11, 2017

shiftedreality deleted the MAGECLOUD-1372 branch December 11, 2017 16:04

YPyltiai added Release: 2002.0.8 ECE-Tools Release and removed Release Line: 2002.0 Progress: testing in progress PR/issue status labels May 1, 2019

magento-devops-reposync-svc pushed a commit that referenced this pull request Jul 18, 2023

MCLOUD-10250: update broken onboarding docs link in email template (#115

34b0dd7

) MCLOUD-10250: update broken onboarding docs link in email template

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MAGECLOUD-1372: Backup Critical Configuration Files to Recover On-Demand #115

MAGECLOUD-1372: Backup Critical Configuration Files to Recover On-Demand #115

BaDos commented Dec 8, 2017 •

edited

Loading

oshmyheliuk Dec 8, 2017

BaDos Dec 8, 2017

oshmyheliuk Dec 8, 2017

BaDos Dec 8, 2017

oshmyheliuk Dec 8, 2017

BaDos Dec 8, 2017

shiftedreality Dec 8, 2017

BaDos Dec 8, 2017

shiftedreality Dec 8, 2017

BaDos Dec 8, 2017 •

edited

Loading

shiftedreality Dec 8, 2017

BaDos Dec 8, 2017

NadiyaS Dec 8, 2017

NadiyaS Dec 8, 2017

BaDos Dec 8, 2017

NadiyaS Dec 8, 2017

BaDos Dec 8, 2017

NadiyaS Dec 8, 2017

BaDos Dec 8, 2017

NadiyaS Dec 8, 2017 •

edited

Loading

BaDos Dec 8, 2017

NadiyaS Dec 8, 2017

BaDos Dec 8, 2017

NadiyaS Dec 11, 2017

BaDos Dec 11, 2017

BaDos Dec 11, 2017

oshmyheliuk Dec 11, 2017

oshmyheliuk Dec 11, 2017

oshmyheliuk Dec 11, 2017

BaDos Dec 11, 2017

oshmyheliuk Dec 11, 2017

NadiyaS Dec 11, 2017

BaDos commented Dec 11, 2017

andriyShevtsov commented Dec 11, 2017

MAGECLOUD-1372: Backup Critical Configuration Files to Recover On-Demand #115

MAGECLOUD-1372: Backup Critical Configuration Files to Recover On-Demand #115

Conversation

BaDos commented Dec 8, 2017 • edited Loading

Description

Fixed Issues (if relevant)

Zephyr Tests

Contribution checklist

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

BaDos Dec 8, 2017 • edited Loading

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

NadiyaS Dec 8, 2017 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BaDos commented Dec 11, 2017

andriyShevtsov commented Dec 11, 2017

BaDos commented Dec 8, 2017 •

edited

Loading

BaDos Dec 8, 2017 •

edited

Loading

NadiyaS Dec 8, 2017 •

edited

Loading