Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Patch command + Configuration class should not continually create schema #60

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
4 participants
  1. Patch command added to console:

Implemented PatchCommand to execute any missing migrations up to the current version. This has been useful for us when needing to merge branches an application that uses doctrine

  1. Do not continually create schema after migration table has been created to avoid long running migrate scripts:

migrationTableCreated flag needs to be set in Configuration class after we have checked that the migration table exists. This will that ensure a new schema is not created for each version that we migrate to thereafter.

Tom and others added some commits Sep 12, 2011

do not continually create schema after migration table has been creat…
…ed to avoid long running migrate scripts

migrationTableCreated flag needs to be set in Configutation class after we have checked that the migration table exists.
This will that ensure a new schema is not created for each version that we migrate to thereafter.

@stof stof commented on an outdated diff Feb 24, 2012

...trine/DBAL/Migrations/Configuration/Configuration.php
*/
public function createMigrationTable()
{
- $this->validate();
+ if ($this->migrationTableCreated)
+ return;
@stof

stof Feb 24, 2012

Member

you need to add the curly braces

@stof stof commented on an outdated diff Feb 24, 2012

...trine/DBAL/Migrations/Configuration/Configuration.php
@@ -406,15 +406,14 @@ public function getLatestVersion()
/**
* Create the migration table to track migrations with.
*
- * @return bool $created Whether or not the table was created.
+ * @return void
@stof

stof Feb 24, 2012

Member

remove it entirely instead of using @return void. void has no meaning in PHP

@stof stof commented on an outdated diff Feb 24, 2012

...trine/DBAL/Migrations/Configuration/Configuration.php
@@ -457,6 +453,28 @@ public function getMigrationsToExecute($direction, $to)
}
return $versions;
}
+
+ /**
+ * Returns the array of migrations that have not be run up to the
+ * specified version.
+ *
+ * @param string $to The version to migrate to.
+ * @return array $migrations The array of migrations we can execute.
+ */
+ public function getMissingMigrations($to)
+ {
+ $versions = array();
+
+ foreach($this->migrations as $version)
+ {
@stof

stof Feb 24, 2012

Member

the curly brace should be on the previous line

@stof stof commented on an outdated diff Feb 24, 2012

...trine/DBAL/Migrations/Configuration/Configuration.php
+
+ /**
+ * Returns the array of migrations that have not be run up to the
+ * specified version.
+ *
+ * @param string $to The version to migrate to.
+ * @return array $migrations The array of migrations we can execute.
+ */
+ public function getMissingMigrations($to)
+ {
+ $versions = array();
+
+ foreach($this->migrations as $version)
+ {
+ if ($this->hasVersionMigrated($version) || $version->getVersion() > $to)
+ continue;
@stof

stof Feb 24, 2012

Member

missing curly braces

@stof stof commented on an outdated diff Feb 24, 2012

...BAL/Migrations/Tools/Console/Command/PatchCommand.php
+ ->addOption('write-sql', null, InputOption::VALUE_NONE, 'The path to output the migration SQL file instead of executing it.')
+ ->addOption('dry-run', null, InputOption::VALUE_NONE, 'Execute the migration as a dry run.')
+ ->setHelp(<<<EOT
+The <info>%command.name%</info> command executes any missing migrations up to current version:
+
+ <info>%command.full_name%</info>
+
+You can also execute the migration as a <comment>--dry-run</comment>:
+
+ <info>%command.full_name% YYYYMMDDHHMMSS --dry-run</info>
+
+You can output the would be executed SQL statements to a file with <comment>--write-sql</comment>:
+
+ <info>%command.full_name% YYYYMMDDHHMMSS --write-sql</info>
+
+Or you can also execute the migration without a warning message wich you need to interact with:
@stof

stof Feb 24, 2012

Member

typo here: wich => which

@stof stof commented on an outdated diff Feb 24, 2012

...BAL/Migrations/Tools/Console/Command/PatchCommand.php
+ parent::configure();
+ }
+
+ public function execute(InputInterface $input, OutputInterface $output)
+ {
+ $configuration = $this->getMigrationConfiguration($input, $output);
+ $patch = new Patch($configuration);
+
+ $this->outputHeader($configuration, $output);
+
+ if ($path = $input->getOption('write-sql')) {
+ $path = is_bool($path) ? getcwd() : $path;
+ $patch->writeSqlFile($path);
+ } else {
+ $dryRun = $input->getOption('dry-run') ? true : false;
+ if ($dryRun === true) {
@stof

stof Feb 24, 2012

Member

simply if ($dryRun) {

@stof stof commented on an outdated diff Feb 24, 2012

...BAL/Migrations/Tools/Console/Command/PatchCommand.php
+ {
+ $configuration = $this->getMigrationConfiguration($input, $output);
+ $patch = new Patch($configuration);
+
+ $this->outputHeader($configuration, $output);
+
+ if ($path = $input->getOption('write-sql')) {
+ $path = is_bool($path) ? getcwd() : $path;
+ $patch->writeSqlFile($path);
+ } else {
+ $dryRun = $input->getOption('dry-run') ? true : false;
+ if ($dryRun === true) {
+ $patch->patch(true);
+ } else {
+ $noInteraction = $input->getOption('no-interaction') ? true : false;
+ if ($noInteraction === true) {
@stof

stof Feb 24, 2012

Member

same here

@stof stof commented on an outdated diff Feb 24, 2012

...BAL/Migrations/Tools/Console/Command/PatchCommand.php
+ $this->outputHeader($configuration, $output);
+
+ if ($path = $input->getOption('write-sql')) {
+ $path = is_bool($path) ? getcwd() : $path;
+ $patch->writeSqlFile($path);
+ } else {
+ $dryRun = $input->getOption('dry-run') ? true : false;
+ if ($dryRun === true) {
+ $patch->patch(true);
+ } else {
+ $noInteraction = $input->getOption('no-interaction') ? true : false;
+ if ($noInteraction === true) {
+ $patch->patch($dryRun);
+ } else {
+ $confirmation = $this->getHelper('dialog')->askConfirmation($output, '<question>WARNING! You are about to execute a database migration that could result in schema changes and data lost. Are you sure you wish to continue? (y/n)</question>', 'y');
+ if ($confirmation === true) {
@stof

stof Feb 24, 2012

Member

and here

@stof stof commented on an outdated diff Feb 24, 2012

...mfony/Component/Console/Formatter/OutputFormatter.php
@@ -0,0 +1,190 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Console\Formatter;
@stof

stof Feb 24, 2012

Member

Please don't put the code of the Console component in this repo. there is no valid reason to copy-paste it in this library

@stof stof commented on an outdated diff Feb 24, 2012

lib/vendor/Symfony/Component/Yaml/Escaper.php
@@ -0,0 +1,88 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\Yaml;
@stof

stof Feb 24, 2012

Member

same for the Yaml component

Tom Maguire added some commits Mar 3, 2012

upgraded Console and Yaml components and made them submodules
Conflicts:

	lib/vendor/Symfony/Component/Console
	lib/vendor/Symfony/Component/Yaml

@tomdmaguire tomdmaguire closed this Mar 3, 2012

@tomdmaguire tomdmaguire reopened this Mar 3, 2012

requested changes have been made

@stof stof and 1 other commented on an outdated diff Mar 3, 2012

lib/Doctrine/DBAL/Migrations/Patch.php
+ */
+ public function __construct(Configuration $configuration)
+ {
+ $this->configuration = $configuration;
+ $this->outputWriter = $configuration->getOutputWriter();
+ }
+
+ /**
+ * Get the array of versions and SQL queries that would be executed for
+ * each version but do not execute anything.
+ *
+ * @return array $sql The array of SQL queries.
+ */
+ public function getSql()
+ {
+ //TODO
@stof

stof Mar 3, 2012

Member

this should be done before merging

@tomdmaguire

tomdmaguire Jun 28, 2012

added to my master branch (8eeebc3)

@stof stof commented on an outdated diff Mar 3, 2012

...BAL/Migrations/Tools/Console/Command/PatchCommand.php
+ public function execute(InputInterface $input, OutputInterface $output)
+ {
+ $configuration = $this->getMigrationConfiguration($input, $output);
+ $patch = new Patch($configuration);
+
+ $this->outputHeader($configuration, $output);
+
+ if ($path = $input->getOption('write-sql')) {
+ $path = is_bool($path) ? getcwd() : $path;
+ $patch->writeSqlFile($path);
+ } else {
+ $dryRun = $input->getOption('dry-run') ? true : false;
+ if ($dryRun) {
+ $patch->patch(true);
+ } else {
+ $noInteraction = $input->getOption('no-interaction') ? true : false;
@stof

stof Mar 3, 2012

Member

$noInteraction = $input->getOption('no-interaction') will do the same

@stof stof and 1 other commented on an outdated diff Mar 3, 2012

...BAL/Migrations/Tools/Console/Command/PatchCommand.php
+ if ($dryRun) {
+ $patch->patch(true);
+ } else {
+ $noInteraction = $input->getOption('no-interaction') ? true : false;
+ if ($noInteraction) {
+ $patch->patch($dryRun);
+ } else {
+ $confirmation = $this->getHelper('dialog')->askConfirmation($output, '<question>WARNING! You are about to execute a database migration that could result in schema changes and data lost. Are you sure you wish to continue? (y/n)</question>', 'y');
+ if ($confirmation) {
+ $patch->patch($dryRun);
+ } else {
+ $output->writeln('<error>Patch cancelled!</error>');
+ }
+ }
+ }
+ }
@stof

stof Mar 3, 2012

Member

All this could be simplified:

<?php
    public function execute(InputInterface $input, OutputInterface $output)
    {
        $configuration = $this->getMigrationConfiguration($input, $output);
        $patch = new Patch($configuration);

        $this->outputHeader($configuration, $output);

        if ($path = $input->getOption('write-sql')) {
            $path = is_bool($path) ? getcwd() : $path;
            $patch->writeSqlFile($path);
        } elseif ($input->getOption('dry-run') || $input->getOption('no-interaction')) {
            $patch->patch(true);
        } else {
            $confirmation = $this->getHelper('dialog')->askConfirmation($output, '<question>WARNING! You are about to execute a database migration that could result in schema changes and data lost. Are you sure you wish to continue? (y/n)</question>', 'y');
            if ($confirmation) {
                $patch->patch(false);
            } else {
                $output->writeln('<error>Patch cancelled!</error>');
            }
        }
    }
@tomdmaguire

tomdmaguire Jun 28, 2012

added in my master branch (8eeebc3)

suggested changes made, thanks

Hi there, any chance this could be merged in?

👍

Member

mikeSimonson commented Feb 14, 2015

@tomdmaguire As far as I can see both feature that you propose in this PR is the way the current doctrine migration function right ?

Are there other stuff in this PR that need to be pulled in or can it be closed ?

Yeah, I believe this got implemented

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