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

[WIP] Реалізація міграцій БД з використанням doctrine migrations #74

Open
wants to merge 48 commits into
base: master
Choose a base branch
from

Conversation

yukoff
Copy link
Contributor

@yukoff yukoff commented Jul 18, 2017

Щоб в майбутньому бути сумісним з laravel (upstream TP перейшов на нього в experimental
гілці) почнімо з міграцій doctrine.

Через використання ENUM довелось задіяти ORM та згенерувати відповідні entities, плюс необхідно проаналізувати, чи можливий т.зв. data-seeding у випадку doctrine (якщо замість альтерів таблиць з даними переливати дані зі старої структури в нову, пусту базу)

  • cli-додаток для роботи з міграціями - використовується artisan з laravel (upstream переходять на нього)
  • міграція з TP v2.2.1 на v2.2.2, якщо контейнер вже оновлювався - міграція не повинна "падати"
  • конвертація бази в utf8mb4/utf8mb4_unicode_ci
  • кастомні типи даних doctrine для ENUM типу даних
  • використання utf8mb4_bin для зберігання паролів, ip-адрес, ідентифікаторів сесій тощо - частково це було реалізовано до пропонованих змін
  • проаналізувати можливість data-seeding у випадку doctrine - laravel може сідування даних
  • спробувати замість класів з анотаціями метадані зберігати в .yml
  • змінити простір імен для класів ORM, налаштувати їх автозавантаження в composer.json

Closes #7, closes #46, closes #76

TODO

@yukoff yukoff self-assigned this Jul 18, 2017
@yukoff yukoff requested a review from konfuciusu July 18, 2017 07:47
@yukoff yukoff added this to the v3.0.0 milestone Jul 18, 2017
@yukoff yukoff added pr and removed pr labels Jul 18, 2017
@konfuciusu
Copy link
Contributor

використання utf8mb4_bin для зберігання паролів, ip-адрес, ідентифікаторів сесій тощо - частково це було реалізовано до пропонованих змін

Добре, що не набули про це при переході на unicode_ci 👍

@yukoff yukoff force-pushed the feature/database-migrations branch from 6e06b39 to 0beffd0 Compare July 23, 2017 06:10
@konfuciusu
Copy link
Contributor

konfuciusu commented Jul 26, 2017

Наївна реалізація завдання про docker-compose:

toloka-migration:
    image: hurtom/php:nginx
    command: bash -c "sleep 30 && doctrine migration command"
    volumes:
      - ./:/var/www/html
    env_file:
      - dev.env

На жаль, автори docker відкинули docker/compose#374 і docker/compose#1809, тому напевне єдина альтернатива - це додати entrypoint-скрипт, але тоді варто звернути увагу на docker/compose#3140

@konfuciusu
Copy link
Contributor

Можна ще за допомогою supervisord:

[program:migration]
command = doctrine migration command
autorestart = false
stdout_logfile = /dev/stdout
stdout_logfile_maxbytes = 0
stderr_logfile = /dev/stderr
stderr_logfile_maxbytes = 0

@yukoff
Copy link
Contributor Author

yukoff commented Jul 26, 2017

Про supervisord я й не подумав. В принципі, мені головне включити їх до процесу тестування, та й +1 рядок в документації теж не проблема :) Можна мабуть через окремий issue/PR. Зараз аналізую/адаптую код з #76, перебазований над цим - все ніяк до docker-compose не доберусь.

@konfuciusu
Copy link
Contributor

Зараз ще згадав: doctrine migration command бажано загорнути в невеликий скрипт (php/bash), який запускає комадну аж тоді, коли mariadb стане доступною.

@konfuciusu
Copy link
Contributor

Таблиці, які не позначані ***, можна пропустити:

bb_bt_config
bb_bt_search_results
bb_bt_torrents_del
bb_confirm
bb_dl_st_prevs_list
bb_easymod
bb_easymod_processed_files
bb_flags
bb_forbidden_extensions
bb_forum_prune
bb_search_wordlist
bb_search_wordmatch
bb_sessions_keys
bb_themes
bb_themes_name
bb_topics_move

Вони не мають жодної цінності.

@yukoff
Copy link
Contributor Author

yukoff commented Aug 3, 2017

@konfuciusu Трохи не вловив - а позначені де? Тобто цей перелік

bb_bt_config
bb_bt_search_results
bb_bt_torrents_del
bb_confirm
bb_dl_st_prevs_list
bb_easymod
bb_easymod_processed_files
bb_flags
bb_forbidden_extensions
bb_forum_prune
bb_search_wordlist
bb_search_wordmatch
bb_sessions_keys
bb_themes
bb_themes_name
bb_topics_move

можна викидати?

@konfuciusu
Copy link
Contributor

Тут: https://github.com/hurtom/toloka/blob/master/install/sql/toloka_old.sql

Так, можна викидати.

@yukoff
Copy link
Contributor Author

yukoff commented Aug 4, 2017

@konfuciusu Аааа... Вибачаюсь, я не звернув увагу на ті "зірочки" в старій схемі 🤦‍♂️ 😄

yukoff added 14 commits August 20, 2017 06:35
This one is to reflect changes from PR hurtom#53 and to be applied during
database migration
* Add migration for bb_bt_torrents.topic_check* (hurtom#43)
* Aggregate ALTERs to reduce table modifications
* Source formatting, reordered steps alphabetically
* Fetch via iterator instead of fetchAll()
* Convert rest of tables to InnoDB
TIMESTAMP fields should have set {version: true} in field description.
Also make corresponding fields timestampable via gedmo/doctrine-extensions.
Make quoting ENUM values the right way
* Reorder clauses for readability
* Adjust cat_id datatype, bb_categories and related tables
* Adjust forum_id datatype
* Adjust topic_id and related
* Adjust post_id and related
* Adjust group_id and related
* Adjust warning_id and related
Follow up review comments in PR hurtom#53, add `thanks_count` column
Migration should succeed with either way of migration - from the old 0.x
engine to current or from the stock 2.x schema.

Reset some table options, eg. MAX_ROWS and ROW_FORMAT.

Reflect changes for TP v2.2.0 (at this point all `*_ip` columns were
reset to '0', drop bb_ads table etc) and v2.2.2

Adjust datatypes to comply with doctrine etc
@yukoff yukoff force-pushed the feature/database-migrations branch from fea31a2 to 7be1b3f Compare August 20, 2017 03:37
@yukoff
Copy link
Contributor Author

yukoff commented Aug 23, 2017

Я тут думаю - може варто замість автоматичного запуску через docker-compose up -d все ж робити це за допомогою composer

diff --git a/composer.json b/composer.json
index ff0de5d2..ecdfdcfa 100644
--- a/composer.json
+++ b/composer.json
@@ -88,6 +88,9 @@
       "Illuminate\\Foundation\\ComposerScripts::postUpdate",
       "php artisan optimize"
     ],
+    "database:migrate": [
+      "php artisan doctrine:migrations:migrate --force"
+    ],
     "test": [
       "echo 'NOTE: Instead of running <composer test:lint> linting should be done with StyleCI'",
       "composer test:unit",

та відповідно

docker-compose up -d
docker-compose exec -u www-data toloka composer update -o
docker-compose exec -u www-data toloka composer database:migrate

Або можна навіть database:migrate додати у post-update-cmd, тоді остання команда не потрібна

@yukoff yukoff force-pushed the feature/database-migrations branch 3 times, most recently from 854d5ce to 87999c2 Compare August 23, 2017 19:43
yukoff added 3 commits August 23, 2017 22:53
* Update `codeception/c3`
* Update `laravel-doctrine/extensions` (to min. version with support for
  laravel 5.4)
* Update `laravel-doctrine/migrations` (to min. version supporting
  laravel 5.4)
* Update `laravel-doctrine/orm`
* Add redis sample config (.env)
* Use `file` as a default cache driver
Add `database:migrate` composer command and run it as part of travis `before_script`
Also use username instead of user id ("magic number")
* Merge remaining queries, reorder and add references to issues/PRs
* Adjust datatype for topic_type
* Add validation for torrent check status
* Some source formatting etc
@konfuciusu
Copy link
Contributor

Через composer database:migrate теж нормально, треба тільки це тоді додати в readme.

Можна вже пробувати тестувати на старій базі?

@yukoff
Copy link
Contributor Author

yukoff commented Sep 22, 2017

@konfuciusu Майже, є кілька моментів які зловив при перебазуванні #55 - днями з ними розберусь, думаю десь на наступному тижні вже можна буде тестувати.

В принципі міграція працює вже зараз, але після міграції не все працює з функціоналу через нові поля і відсутність типових значень на них. Зараз можна хіба що прогнати тестову міграцію на знятку (снепшоті), аби прикинути необхідний час і можливо щось оптимізувати.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment