Separate db tool and db backup/restore services for version 2.x #398

alekseytupichenkov · 2018-02-14T09:03:33Z

Provide services for backup/restore different database and refactoring code

alekseytupichenkov · 2018-02-14T10:12:28Z

@alexislefebvre
Also, I have suggestion, maybe need remove liip_functional_test.cache_sqlite_db at all? Currently we have liip_functional_test.cache_db.* to make this

alexislefebvre · 2018-02-15T09:47:26Z

There is a conflict since #393 has been merged, sorry.

Thanks for the amazing work, it looks really nice!

I think that we can drop liip_functional_test.cache_sqlite_db if you added a replacement, we don't need to keep a BC parameter on the 2.x branch.

alexislefebvre · 2018-02-15T09:48:06Z

src/Services/DatabaseBackup/MysqlCustomDatabaseBackup.php

+
+        $executor->getReferenceRepository()->save($this->getBackupName());
+
+        exec("mysqldump -h $dbHost -u $dbUser -p$dbPass $dbName > {$this->getBackupName()}");


If the MySQL database is in a Docker container, this command won't work?

I use docker too, it will works fine if you install mysql-client in container with symfony app :)

ghost · 2018-02-16T07:26:48Z

I'll resolve conflicts in this weekend

…ichenkov/LiipFunctionalTestBundle into db_tools_from_2.x # Conflicts: # src/Test/WebTestCase.php

alekseytupichenkov · 2018-02-18T16:50:57Z

@alexislefebvre done :)
Also updated pr for 1.9.x

alexislefebvre · 2018-02-18T17:18:12Z

Can you please explain why you call mysqldump? I don't understand why you chose to rely on a CLI command, the current version of bundle only rely on PHP code and I prefer this option personnally.

Otherwise, this PR is really good.

@Jean85 @lsmith77 Could you please give your opinion about this PR ? Thanks.

alekseytupichenkov · 2018-02-18T18:20:54Z

I use mysqldump only to create sql dump file, I didn't find this functional in Doctrine. Maybe someone can help with this?
However, MysqlCustomDatabaseBackup.php just an example for how it can work, anyone can create their own service for make backup/restore using their tools

alexislefebvre · 2018-02-18T22:33:50Z

I think that the backup is done with the save() function:

LiipFunctionalTestBundle/src/Test/WebTestCase.php

Line 435 in afddaec

$executor->getReferenceRepository()->save($backup);

Here is the method: https://github.com/doctrine/data-fixtures/blob/318df610720a7a7de0add8b27107654877642ac4/lib/Doctrine/Common/DataFixtures/ProxyReferenceRepository.php#L115

There is a reference to LiipFunctionalTestBundle in the documentation of this class: https://github.com/doctrine/data-fixtures/blob/318df610720a7a7de0add8b27107654877642ac4/lib/Doctrine/Common/DataFixtures/ProxyReferenceRepository.php#L115

lsmith77 · 2018-02-19T14:39:25Z

README.md

+    # app/config/config_test.yml
+    liip_functional_test:
+        cache_db:
+            sqlite: liip_functional_test.database_backup.sqllite


Yeah, it's typo

lsmith77 · 2018-02-19T14:40:05Z

src/DependencyInjection/Configuration.php

@@ -31,6 +31,15 @@ public function getConfigTreeBuilder(): TreeBuilder
        $rootNode
            ->children()
                ->booleanNode('cache_sqlite_db')->defaultFalse()->end()
+                ->arrayNode('cache_db')


should we then remove cache_sqlite_db ?

Yes, I'll remove it.
#398 (comment)

lsmith77 · 2018-02-19T14:41:35Z

src/Services/DatabaseBackup/MysqlCustomDatabaseBackup.php

+        $em = $executor->getReferenceRepository()->getManager();
+        self::$metadata = $em->getMetadataFactory()->getLoadedMetadata();
+
+        exec("mysqldump -h $dbHost -u $dbUser -p$dbPass --no-create-db $dbName > {$this->getBackupName()}");


#398 (comment)

@alexislefebvre @lsmith77
About #398 (comment), it's not suit, this method save only reference data, but I need save database data (sql with inserts data per each table)

Indeed ..

Generally I think if we have to use a CLI command, then its not really a good fit for this Bundle, since it will not be applicable in too many cases.

This library here might help however https://github.com/portphp/doctrine

Hmm... I added mysql class just for example
I can use portphp/doctrine or something else, it will be interesting for me, but I have fears about performance and additional vendor...

Currently I rename service mysql_custom -> mysql_example and add attention block from readme

As a proposal, mysql (and other) can be implemented by a separate repo in github, what do you think about it? @alexislefebvre @lsmith77

@alekseytupichenkov

About #398 (comment), it's not suit, this method save only reference data, but I need save database data (sql with inserts data per each table)

The method works on the current version of this bundle. I'm sorry but I don't understand why you don't use it in this PR. Could you please explain why you chose to perform a full export instead of using only the reference data?

@alexislefebvre I save reference too
https://github.com/liip/LiipFunctionalTestBundle/pull/398/files#diff-f2e540017ed6e7479f8d4a4b1e17a480R82

But it's not enough, I need backup both, DB and reference.
In case of SQLite, to copy DB enough copy just file, in case of MySql we can't just copy files, need make backup, for make backup I use mysqldump because Doctrine can't do it

@lsmith77 about https://github.com/portphp/doctrine

This lib really cool, but still not suitable, this lib saves only entities, but I need dump full db.
For example, after loading fixtures I can have some additional data, by reason to Doctrine lifecycle methods or DB triggers

I found some PHP libs to make backup like https://github.com/backup-manager/backup-manager or https://github.com/spatie/db-dumper
But they all use mysqldump too

Thanks for your explanation, I was confused by the fact that SQLite and MySQL need different ways to perform backups. I understand now. Thanks.

lsmith77 · 2018-02-19T14:42:49Z

didn't do a detailed code review .. but yes splitting this out is very good and fits our goals from #392

update readme fix sqlite typo rename mysql_custom -> mysql_example

alexislefebvre · 2018-02-26T19:56:50Z

It looks like mysql dumps are causing headaches: https://github.com/liip/LiipFunctionalTestBundle/pull/398/files/831b101c3d9f752c5dfa3eb38799e7d5cf260fb2#r169751346

We may merge this PR without the MySQL backup and add it later in another PR? Do you agree @alekseytupichenkov? If it's OK for you, you can finalize this PR, move the changes to another branch and open another PR.

ghost · 2018-02-27T08:02:24Z

@alexislefebvre Yeah, it's ok, I'll make new PR soon

rename method getBackupName -> getBackupFilePath update readme

alekseytupichenkov · 2018-03-03T14:19:33Z

@alexislefebvre done, also made same in #397

alexislefebvre

Thanks for the amazing work!

alekseytupichenkov · 2018-03-04T18:47:35Z

Added separate PRs with database cache services (mysql and mongodb)

lsmith77 · 2018-03-05T21:11:21Z

only did a very quick review but looks great!

one thing to note in general, we probably should create an UPGRADING.md file to document all the BC breaks and how to migrate existing code to the new version.

lsmith77 · 2018-03-06T22:00:45Z

ah .. totally overlooked we already have such a file. thank you for adding .. all good from my side then.

lsmith77 · 2018-03-06T22:11:48Z

really awesome work .. and thank you very much for being so receptive on the feedback you got.

alexislefebvre · 2018-03-06T22:41:01Z

Thanks @alekseytupichenkov for the amazing work and @lsmith77 for the review and merge. 🎉

I published a new release with these changes: https://github.com/liip/LiipFunctionalTestBundle/releases/tag/2.0.0-alpha4

soullivaneuh · 2018-05-22T08:46:55Z

This PR cause troubles on some configurations. Please see #423.

Separate db tool and db backup/restore services for version 2.x

02e3518

alekseytupichenkov force-pushed the db_tools_from_2.x branch from 446f9af to 02e3518 Compare February 14, 2018 09:53

code style fix

fb40d39

alekseytupichenkov mentioned this pull request Feb 14, 2018

Separate db tool and db backup/restore services #396

Closed

alexislefebvre reviewed Feb 15, 2018

View reviewed changes

alekseytupichenkov added 4 commits February 18, 2018 17:21

Merge branches '2.x' and 'db_tools_from_2.x' of github.com:alekseytup…

c92e7e4

…ichenkov/LiipFunctionalTestBundle into db_tools_from_2.x # Conflicts: # src/Test/WebTestCase.php

Changes from pr liip#393

260d01c

fixes and MysqlCustomDatabaseBackup boost

5923bed

fix when append fixtures

66e4390

alexislefebvre added the RFC label Feb 18, 2018

alexislefebvre added this to the 2.0 milestone Feb 18, 2018

alexislefebvre mentioned this pull request Feb 18, 2018

[WIP] Move fixtures loading in a service #395

Closed

cache schema too, it's more predictable

831b101

lsmith77 reviewed Feb 19, 2018

View reviewed changes

alekseytupichenkov added 5 commits February 20, 2018 09:02

performance update

817c8ee

fix typo

1a71772

refactoring

6379ac1

update readme fix sqlite typo rename mysql_custom -> mysql_example

removed cache_sqlite_db from config

c355de3

fix mongodb service name

7d2d90e

alexislefebvre mentioned this pull request Feb 26, 2018

Discussion about 2.0 release #381

Closed

12 tasks

alexislefebvre removed the RFC label Mar 2, 2018

alekseytupichenkov added 2 commits March 3, 2018 15:33

remove MysqlExampleDatabaseBackup

966681b

rename method getBackupName -> getBackupFilePath update readme

Travis trigger

87d9f1a

alexislefebvre approved these changes Mar 3, 2018

View reviewed changes

small refactoring and fixes

b919a90

alekseytupichenkov force-pushed the db_tools_from_2.x branch from 92ae157 to b919a90 Compare March 4, 2018 18:13

alekseytupichenkov mentioned this pull request Mar 4, 2018

database cache from 2.x #402

Merged

alexislefebvre approved these changes Mar 5, 2018

View reviewed changes

Added instruction to upgrade from 1.x to 2.0

3cde299

lsmith77 merged commit 321db83 into liip:2.x Mar 6, 2018

alexislefebvre mentioned this pull request Mar 6, 2018

[2.0] Reduce size of WebTestCase? #392

Closed

soullivaneuh mentioned this pull request May 18, 2018

Conflict with dama/doctrine-test-bundle #423

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separate db tool and db backup/restore services for version 2.x #398

Separate db tool and db backup/restore services for version 2.x #398

alekseytupichenkov commented Feb 14, 2018

alekseytupichenkov commented Feb 14, 2018 •

edited

Loading

alexislefebvre commented Feb 15, 2018

alexislefebvre Feb 15, 2018

ghost Feb 16, 2018

ghost commented Feb 16, 2018

alekseytupichenkov commented Feb 18, 2018

alexislefebvre commented Feb 18, 2018

alekseytupichenkov commented Feb 18, 2018

alexislefebvre commented Feb 18, 2018

lsmith77 Feb 19, 2018

alekseytupichenkov Feb 20, 2018

lsmith77 Feb 19, 2018

alekseytupichenkov Feb 20, 2018

lsmith77 Feb 19, 2018

alekseytupichenkov Feb 20, 2018

lsmith77 Feb 20, 2018

alekseytupichenkov Feb 21, 2018

alexislefebvre Feb 21, 2018

alekseytupichenkov Feb 21, 2018

alekseytupichenkov Feb 21, 2018

alexislefebvre Feb 26, 2018

lsmith77 commented Feb 19, 2018

alexislefebvre commented Feb 26, 2018 •

edited

Loading

ghost commented Feb 27, 2018

alekseytupichenkov commented Mar 3, 2018

alexislefebvre left a comment

alekseytupichenkov commented Mar 4, 2018

lsmith77 commented Mar 5, 2018

lsmith77 commented Mar 6, 2018

lsmith77 commented Mar 6, 2018

alexislefebvre commented Mar 6, 2018

soullivaneuh commented May 22, 2018


		$executor->getReferenceRepository()->save($this->getBackupName());

		exec("mysqldump -h $dbHost -u $dbUser -p$dbPass $dbName > {$this->getBackupName()}");

Separate db tool and db backup/restore services for version 2.x #398

Separate db tool and db backup/restore services for version 2.x #398

Conversation

alekseytupichenkov commented Feb 14, 2018

alekseytupichenkov commented Feb 14, 2018 • edited Loading

alexislefebvre commented Feb 15, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Feb 16, 2018

alekseytupichenkov commented Feb 18, 2018

alexislefebvre commented Feb 18, 2018

alekseytupichenkov commented Feb 18, 2018

alexislefebvre commented Feb 18, 2018

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

lsmith77 commented Feb 19, 2018

alexislefebvre commented Feb 26, 2018 • edited Loading

ghost commented Feb 27, 2018

alekseytupichenkov commented Mar 3, 2018

alexislefebvre left a comment

Choose a reason for hiding this comment

alekseytupichenkov commented Mar 4, 2018

lsmith77 commented Mar 5, 2018

lsmith77 commented Mar 6, 2018

lsmith77 commented Mar 6, 2018

alexislefebvre commented Mar 6, 2018

soullivaneuh commented May 22, 2018

alekseytupichenkov commented Feb 14, 2018 •

edited

Loading

alexislefebvre commented Feb 26, 2018 •

edited

Loading