Manage backups #57

GregLeBarbar · 2017-10-29T05:48:50Z

From issue: #53

high level:

new commands backup and backup-many to backup one single site or all sites in source of trust.

 $ python jahia2wp.py backup $WP_ENV http://localhost
 ...
 $ python jahia2wp.py backup-many path/to/csv
 ...

Backups are full by default, but can be incremental by using option --backup-type

 $ python jahia2wp.py backup $WP_ENV http://localhost --backup-type=inc
 ...

new environment variable BACKUP_PATH to define where to store backups (by default: jahia2wp/data/backups)

low level:

A backup of a WordPress site relies on a WPConfig, and is called/used in a similar way as WPGenerator. It creates three files:

a .tar of all WP files (php, assets, media)
a .list reference file for incremental backups
a .sql dump of the database

Backups are stored in BACKUP_PATH (by default: jahia2wp/data/backups), with the following name convention: <wp_site_name>[_<timestamp>]_fullN[_incM].<tar|list|sql>

DISCLAIMER: next iteration might revise this structure for something more easily exploitable, such as:

backups
├── site_1
│   ├── 2017-10-31
│   │   ├── full-201710310945.tar
│   │   ├── inc-201711012300.tar
│   │   ├── inc-201711022301.tar
│   │   └── inc-201711032310.tar
│   └── 2017-11-06
│       └── full-201711060946.tar
├── site_2
│   └── 2017-11-06
│       ├── full-201711060947.tar
│       └── inc-201711060946.tar
└── site_3

LuluTchab · 2017-10-30T07:19:05Z

src/wordpress/models.py

-    def __init__(self, openshift_env, wp_site_url, wp_default_site_title=None):
+    def __init__(self, openshift_env, wp_site_id, wp_site_url, wp_default_site_title=None):
+
+        self. wp_site_id = wp_site_id


Y'a un espace de trop là non ?

LuluTchab · 2017-10-30T07:21:12Z

src/wordpress/config.py

@@ -65,8 +65,8 @@ def keep_wp_sites(dir_name):
                    yield WPResult(wp_config.wp_site.path, "KO", "", "", "", "", "")

    def run_wp_cli(self, command):
-        cmd = "wp {} --path='{}'".format(command, self.wp_site.path)
-        return Utils.run_command(cmd)
+        WPUtils.run_wp_cli(command, self.wp_site.path)


Faudrait faire un return WPUtils.run_wp_cli(command, sefl.wp_site_path)

Parce que là, tu exécutes 2x la commande, une fois en l'appelant via WPUtils.run_wp_cli et une fois via return Utils.run_command. Quoique le 2e appel sera probablement foireux car il n'y a pas de notion de "path".

LuluTchab · 2017-10-30T07:22:24Z

src/wordpress/backup.py

+        else:
+            self.dir_name = self.wp_site.folder.split("/")[-1]
+
+        # Create a backup folder data/backups/wp_site_id


Détail mais en lisant le commentaire, j'ai compris que la commande créait le dossier de backup... alors que c'est juste le chemin jusqu'à celui-ci qui est généré. La création du dossier en elle-même est faite plus tard

GregLeBarbar · 2017-10-30T07:52:38Z

Oui c'est corrigé ! 2017-10-30 8:19 GMT+01:00 LuluTchab <notifications@github.com>:

…

***@***.**** commented on this pull request. ------------------------------ In src/wordpress/models.py <#57 (comment)>: > @@ -25,7 +33,9 @@ class WPSite: DEFAULT_TITLE = "New WordPress" WP_VERSION = Utils.get_mandatory_env(key="WP_VERSION") - def __init__(self, openshift_env, wp_site_url, wp_default_site_title=None): + def __init__(self, openshift_env, wp_site_id, wp_site_url, wp_default_site_title=None): + + self. wp_site_id = wp_site_id Y'a un espace de trop là non ? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#57 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AExAaBC-MijbhmalIlAshPQPizE76I6zks5sxXhpgaJpZM4QKMxg> .

GregLeBarbar · 2017-10-30T07:53:53Z

Oui c'est bien return WPUtils.run_wp_cli(command, self.wp_site.path) Tes 2 remarques sont sur une partie du code en dehors de backup.py et donc pas testé ... et cela se voit ! J'ai aussi mis en coup de PEP8 2017-10-30 8:21 GMT+01:00 LuluTchab <notifications@github.com>:

…

***@***.**** commented on this pull request. ------------------------------ In src/wordpress/config.py <#57 (comment)>: > @@ -65,8 +65,8 @@ def keep_wp_sites(dir_name): yield WPResult(wp_config.wp_site.path, "KO", "", "", "", "", "") def run_wp_cli(self, command): - cmd = "wp {} --path='{}'".format(command, self.wp_site.path) - return Utils.run_command(cmd) + WPUtils.run_wp_cli(command, self.wp_site.path) Faudrait faire un return WPUtils.run_wp_cli(command, sefl.wp_site_path) Parce que là, tu exécutes 2x la commande, une fois en l'appelant via WPUtils.run_wp_cli et une fois via return Utils.run_command. Quoique le 2e appel sera probablement foireux car il n'y a pas de notion de "path". — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#57 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AExAaGDcqTVX5fjXUk3zkeOgrhCau_3Nks5sxXjogaJpZM4QKMxg> .

GregLeBarbar · 2017-10-30T07:54:34Z

Donc j'ai corrigé, tu peux faire un pull 2017-10-30 8:53 GMT+01:00 Grégory Charmier <gregory.charmier@gmail.com>:

…

Oui c'est bien return WPUtils.run_wp_cli(command, self.wp_site.path) Tes 2 remarques sont sur une partie du code en dehors de backup.py et donc pas testé ... et cela se voit ! J'ai aussi mis en coup de PEP8 2017-10-30 8:21 GMT+01:00 LuluTchab ***@***.***>: > ***@***.**** commented on this pull request. > ------------------------------ > > In src/wordpress/config.py > <#57 (comment)> > : > > > @@ -65,8 +65,8 @@ def keep_wp_sites(dir_name): > yield WPResult(wp_config.wp_site.path, "KO", "", "", "", "", "") > > def run_wp_cli(self, command): > - cmd = "wp {} --path='{}'".format(command, self.wp_site.path) > - return Utils.run_command(cmd) > + WPUtils.run_wp_cli(command, self.wp_site.path) > > Faudrait faire un return WPUtils.run_wp_cli(command, sefl.wp_site_path) > > Parce que là, tu exécutes 2x la commande, une fois en l'appelant via > WPUtils.run_wp_cli et une fois via return Utils.run_command. Quoique le > 2e appel sera probablement foireux car il n'y a pas de notion de "path". > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#57 (review)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AExAaGDcqTVX5fjXUk3zkeOgrhCau_3Nks5sxXjogaJpZM4QKMxg> > . >

GregLeBarbar · 2017-10-30T07:55:19Z

Si c'est pas clair n'hésites pas à modifier ! Merci pour ta relecture ! 2017-10-30 8:22 GMT+01:00 LuluTchab <notifications@github.com>:

…

***@***.**** commented on this pull request. ------------------------------ In src/wordpress/backup.py <#57 (comment)>: > + + BACKUP_ROOT_DIR = "../data/backups/" + REGEX_FULL_NUMBER = ".+full([0-9]+)\.tar$" + REGEX_INC_NUMBER = ".+full{}_inc([0-9]+)\.tar$" + + def __init__(self, wp_site, backup_type): + + self.wp_site = wp_site + self.type = backup_type + + if not self.wp_site.folder: + self.dir_name = "localhost" + else: + self.dir_name = self.wp_site.folder.split("/")[-1] + + # Create a backup folder data/backups/wp_site_id Détail mais en lisant le commentaire, j'ai compris que la commande créait le dossier de backup... alors que c'est juste le chemin jusqu'à celui-ci qui est généré. La création du dossier en elle-même est faite plus tard — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#57 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AExAaCAHVpsIwnfjdLz8rgJxKruFfbUaks5sxXkwgaJpZM4QKMxg> .

LuluTchab · 2017-10-30T15:30:12Z

J'ai regardé pourquoi les checks Travis ne passaient pas et y'a encore plein de choses à modifier pour que "wp_site_id" soit pris en compte partout... y compris modifier les tests Travis.
Ceci est notifié dans la description de la PR mais si on veut merge et que tout continue à fonctionner, autant adapter aussi le reste du code.
Je m'en occupe

codecov · 2017-10-30T15:58:35Z

Codecov Report

Merging #57 into master will increase coverage by 0.7%.
The diff coverage is 87.33%.

@@            Coverage Diff            @@
##           master      #57     +/-   ##
=========================================
+ Coverage   82.88%   83.59%   +0.7%     
=========================================
  Files          20       21      +1     
  Lines        1221     1359    +138     
=========================================
+ Hits         1012     1136    +124     
- Misses        209      223     +14

Impacted Files	Coverage Δ
src/wordpress/__init__.py	`100% <100%> (ø)`	⬆️
src/tests/test_jahia2wp.py	`98.07% <100%> (+0.25%)`	⬆️
src/veritas/tests/test_veritas.py	`100% <100%> (ø)`	⬆️
src/veritas/validators.py	`96.96% <100%> (+0.19%)`	⬆️
src/utils.py	`96.55% <100%> (+0.12%)`	⬆️
src/settings.py	`100% <100%> (ø)`	⬆️
src/veritas/veritas.py	`100% <100%> (ø)`	⬆️
src/wordpress/tests/test_wordpress.py	`100% <100%> (ø)`	⬆️
src/wordpress/themes.py	`85% <100%> (ø)`	⬆️
src/jahia2wp.py	`74.73% <53.33%> (+0.02%)`	⬆️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 06c27dd...9993734. Read the comment docs.

LuluTchab · 2017-10-30T16:00:57Z

Tests Travis fonctionnels après modifications 😀

ebreton · 2017-10-31T15:19:32Z

Après discussion avec @lboatto et @LuluTchab , j'ai "annulé" les 5 derniers commits qui introduisaient le wp_site_id

git reset --soft HEAD~5
git push -f origin feature-wwp-53-backup

…tings

ebreton · 2017-10-31T08:10:56Z

src/jahia2wp.py

+  jahia2wp.py admins        <wp_env> <wp_site_id> <wp_url>  [--debug | --quiet]
+  jahia2wp.py check-one     <wp_env> <wp_site_id> <wp_url>  [--debug | --quiet] [DEPRECATED]
+  jahia2wp.py clean-one     <wp_env> <wp_site_id> <wp_url>  [--debug | --quiet] [DEPRECATED]
+  jahia2wp.py generate-one  <wp_env> <wp_site_id> <wp_url>  [--debug | --quiet] [DEPRECATED]
    [--wp-title=<WP_TITLE> --admin-password=<ADMIN_PASSWORD>]


A priori pas nécessaire de passer le wp_site_id de la sorte.
Complique l'utilisation de la CLI.
On devrait viser la simplification (par ex enlever wp_env 😏 )

ebreton · 2017-10-31T08:15:17Z

src/tests/test_jahia2wp.py

+# /srv/test/localhost/htdocs/;KO;;;;;
+# /srv/test/localhost/htdocs/unittest;ok;http://localhost/unittest;4.8;wp_"""
+#         assert Utils.run_command('python %s inventory %s /srv/test/localhost'
+#                                  % (SCRIPT_FILE, TEST_ENV)).startswith(expected)


pourquoi commenter ?

ebreton · 2017-10-31T08:17:45Z

src/utils.py

+            backup_listed_incremental_file,
+            source_path
+        )
+        logging.debug("Command to generate tar file: {}".format(command))


à priori déjà loggué dans run_command (ligne suivante)

ebreton · 2017-10-31T08:18:52Z

src/utils.py

+            backup_listed_incremental_file,
+            source_path
+        )
+        logging.debug("Command to generate tar file: {}".format(command))


si jamais, les fonctions de log ne doivent pas utilser format, mais passer les arguments de formatage en argument de fonction:

logging.debug("Command to generate tar file: %s", command)

ebreton · 2017-10-31T08:20:20Z

src/wordpress/config.py

+    #                     ",".join([wp_user.username for wp_user in wp_config.admins]),
+    #                 )
+    #             else:
+    #                 yield WPResult(wp_config.wp_site.path, "KO", "", "", "", "", "")


pourquoi commenter ?

ebreton · 2017-10-31T08:25:59Z

src/wordpress/backup.py

+    Backup types:
+    There are 2 types of backup :
+    - full : Full backup.
+    - inc : Incremental backup.


préciser qu'il s'agit juste des fichiers

ebreton · 2017-10-31T08:26:10Z

src/wordpress/backup.py

+    A backup of a WordPress site contains :
+    - copy of all .php file (tar files)
+    - a dump of the database (.sql file)
+


préciser qu'on a les médias

et les assets :)

ebreton · 2017-10-31T08:27:36Z

src/wordpress/backup.py

+    The content difference between incremental tar files are saved in the list file.
+    """
+
+    BACKUP_ROOT_DIR = "../data/backups/"


pas une bonne idée les chemins relatifs dans le code

ebreton · 2017-10-31T08:51:50Z

src/wordpress/backup.py

+
+
+class WPBackup:
+    """


On propose une autre façon de structurer les fichiers:

backups ├── site_id_1 │ ├── 2017-10-31 │ │ ├── full-2017-10-31-09-45.tar │ │ ├── inc-2017-11-01-23-00.tar │ │ ├── inc-2017-11-02-23-01.tar │ │ └── inc-2017-11-03-23-10.tar │ └── 2017-11-06 │ └── full-2017-11-06-09-46.tar ├── site_id_2 │ └── 2017-11-06 │ ├── full-2017-11-06-09-47.tar │ └── inc-2017-11-06-09-46.tar └── site_id_3

Ca évite d'utiliser des numéros (et de les maintenir/générer) et ca simplifie l'exploitation/maintenance

ebreton · 2017-10-31T15:38:00Z

src/wordpress/backup.py

+from utils import Utils
+
+
+class WPBackup:


needs tests

* origin: (33 commits) added doc moved WPTheme/PluginConfig to dedicated files themes/plugin.py Use var defined in 'settings.py' instead of hard-coded string Use site folder as 'wp_site_id' Small bug fix Change way to generate path, add var for plugin source Change function/option name Use wp_site_id instead of site_name (which is not unique) Use WPSite to create instance instead of openshift_env and wp_site_url Small bug fix Workaround for relative path to fix Travis checks Change how class is displayed Change display way PEP8 compliant Add short doc (very short) Add feature to dump plugin configuration Updated configuration for Add-To-Any, fits to new way to do things Change WP tables infos location (now outside class), prepare to use 'wp site id' instead of 'wp site name', PEP8 Use values defined in 'settings.py' and some cosmetic changes Replace way to configure plugin options. No more WPCLI... ...

GregLeBarbar added 2 commits October 29, 2017 06:45

Manage backups

ba58a56

Fix extract WP site folder

0b60bc9

GregLeBarbar requested a review from LuluTchab October 29, 2017 07:32

LuluTchab reviewed Oct 30, 2017

View reviewed changes

Fix some bugs and PEP8

675dd3c

ebreton force-pushed the feature-wwp-53-backup branch from b4f1152 to 675dd3c Compare October 31, 2017 15:08

ebreton added 13 commits October 31, 2017 16:21

do not log twice command

5d55e3e

change description of tar file: all files are tared, not only php ones

a123b22

precision in doc for content of incremental files

80d75f5

not using site_id quite yet. name instead

fef67e5

added tests for generate_tar

3775ff8

using /tmp to generate tar in tests (for travis to pass)

727ff9a

removed relative path for backups. Added WP_PATH & BACKUP_PATH in set…

8243564

…tings

roll run_wp_cli back into WPConfig

6c860fa

same strategy for WPBackup as for WPGenerator for constructor and config

c33a1bc

gitignore backups

99ebe40

added tests and command to backup one site only

d5bdde8

added $BACKUP_PATH to override BACKUP_PATH (in travis)

364f622

generate DB dump directly in proper directory

01c9c22

ebreton approved these changes Oct 31, 2017

View reviewed changes

ebreton added 4 commits October 31, 2017 21:29

added doc

7e6e43e

declare BACKUP_PATH in .env.sample

415fb44

PEP8

613fb2f

do not decrease coverage

f07ae43

ebreton force-pushed the master branch from 44c101f to 05f3617 Compare October 31, 2017 20:57

ebreton and others added 3 commits October 31, 2017 23:27

merged in PR #54

940ee74

Merge branch 'master' into feature-wwp-53-backup

9993734

ebreton merged commit 3120798 into master Nov 2, 2017

ebreton deleted the feature-wwp-53-backup branch November 6, 2017 07:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Manage backups #57

Manage backups #57

GregLeBarbar commented Oct 29, 2017 •

edited by ebreton

Loading

LuluTchab Oct 30, 2017

LuluTchab Oct 30, 2017

LuluTchab Oct 30, 2017

GregLeBarbar commented Oct 30, 2017 via email

GregLeBarbar commented Oct 30, 2017 via email

GregLeBarbar commented Oct 30, 2017 via email

GregLeBarbar commented Oct 30, 2017 via email

LuluTchab commented Oct 30, 2017 •

edited

Loading

codecov bot commented Oct 30, 2017 •

edited

Loading

LuluTchab commented Oct 30, 2017

ebreton commented Oct 31, 2017

ebreton Oct 31, 2017

ebreton Oct 31, 2017

ebreton Oct 31, 2017

ebreton Oct 31, 2017

ebreton Oct 31, 2017

ebreton Oct 31, 2017

ebreton Oct 31, 2017

ebreton Oct 31, 2017

ebreton Oct 31, 2017

ebreton Oct 31, 2017

ebreton Oct 31, 2017

Manage backups #57

Manage backups #57

Conversation

GregLeBarbar commented Oct 29, 2017 • edited by ebreton Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GregLeBarbar commented Oct 30, 2017 via email

GregLeBarbar commented Oct 30, 2017 via email

GregLeBarbar commented Oct 30, 2017 via email

GregLeBarbar commented Oct 30, 2017 via email

LuluTchab commented Oct 30, 2017 • edited Loading

codecov bot commented Oct 30, 2017 • edited Loading

Codecov Report

LuluTchab commented Oct 30, 2017

ebreton commented Oct 31, 2017

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

GregLeBarbar commented Oct 29, 2017 •

edited by ebreton

Loading

LuluTchab commented Oct 30, 2017 •

edited

Loading

codecov bot commented Oct 30, 2017 •

edited

Loading