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

Console command can break setup #16173

Closed
FreekVandeursen opened this issue Jun 15, 2018 · 7 comments
Closed

Console command can break setup #16173

FreekVandeursen opened this issue Jun 15, 2018 · 7 comments
Labels
Component: Framework/Console Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release

Comments

@FreekVandeursen
Copy link

FreekVandeursen commented Jun 15, 2018

Preconditions

  1. Magento 2.2.4
  2. Empty database

Steps to reproduce

  1. Create a module, containing a console command
  2. Use ProductRepositoryInterface in console command constructor
  3. run bin/magento setup:install

Expected result

  1. Setup completing succesfully

Actual result

  1. Database exception, claiming table 'website' does not exist

Analysis

The root cause seems to be in the Magento\Setup\Model\ObjectManagerProvider class, which calls createCliCommands in it's get function. This will break setup when there are any console commands that depend on the database to exist already. I have seen this happen with the ProductRepository for example.

What is the reason for this createCliCommands call? If I just disable that call, the setup will complete succesfully.
It also seems strange that this call is only present in the CLI SAPI, which means functionality is different between CLI or web installation.

Example

<?php
namespace Test\Test\Console\Command;

use Magento\Catalog\Api\ProductRepositoryInterface;
use Symfony\Component\Console\Command\Command;

class TestCommand extends Command
{

    /**
     * @var ProductRepositoryInterface
     */
    private $productRepository;

    public function __construct(
        ProductRepositoryInterface $productRepository,
        string $name = null
    )
    {
        parent::__construct($name);
        $this->productRepository = $productRepository;
    }

    protected function configure()
    {
        $this->setName('test:test')
            ->setDescription('Test command');
        parent::configure();
    }
}
@magento-engcom-team magento-engcom-team added the Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed label Jun 15, 2018
@kkrieger85 kkrieger85 added the Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed label Jun 19, 2018
@will-b
Copy link
Contributor

will-b commented Jun 20, 2018

@FreekVandeursen Think a proxy should be used instead of the product repository interface to avoid a dependency trying to read the 'website' table.
https://devdocs.magento.com/guides/v2.2/extension-dev-guide/proxies.html
Will see if I can find an example tomorrow.

@FreekVandeursen
Copy link
Author

@will-b I agree with you that there are ways of working around this problem. I wonder though if the current behaviour is intended. I think there are 2 ways of looking at this:

  1. Should just including a repository in a constructor lead to a exception on a system which is not installed yet?
  2. Why does the setup ObjectManager try to generate classes for all CLI commands, even those who are not yet available before installation?

@engcom-backlog-nickolas engcom-backlog-nickolas added the Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed label Jul 5, 2018
@engcom-backlog-nickolas
Copy link

@FreekVandeursen, thank you for your report. As your module dependent on catalog module, you need to specify dependency for you module: https://devdocs.magento.com/guides/v2.2/extension-dev-guide/build/module-load-order.html

@FreekVandeursen
Copy link
Author

@engcom-backlog-nickolas I did specify a dependency in my module.xml, that doesn't solve the problem.

@engcom-backlog-nickolas engcom-backlog-nickolas added Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release and removed Progress: needs update labels Jul 5, 2018
@engcom-backlog-nickolas
Copy link

@FreekVandeursen , thank you for your feedback.
We've acknowledged the issue and added to our backlog.

@Bartlomiejsz
Copy link
Contributor

@FreekVandeursen @engcom-backlog-nazar possible that it was duplicated in #21752 which is closed due to #21693

@engcom-Charlie
Copy link
Contributor

Hello @FreekVandeursen

Thank you for contribution and collaboration!

We are not able to reproduce this issue on the lates 2.3-develop branch by provided steps.

We are closing this issue due to branch 2.2-develop was closed and Magento no longer accepts pull requests and issues to the v2.2 release lines to focus all development efforts on v2.3.
See Accepted pull requests and ported code for more details

If you still faced this issue on 2.3 please create a new Issue with all required details according to Issue reporting guidelines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Framework/Console Issue: Clear Description Gate 2 Passed. Manual verification of the issue description passed Issue: Confirmed Gate 3 Passed. Manual verification of the issue completed. Issue is confirmed Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

No branches or pull requests

7 participants