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

[2.2.0-RC1.1] cron:install issues #10040

Closed
hostep opened this issue Jun 24, 2017 · 11 comments
Closed

[2.2.0-RC1.1] cron:install issues #10040

hostep opened this issue Jun 24, 2017 · 11 comments
Assignees
Labels
bug report Component: Config Fixed in 2.3.x The issue has been fixed in 2.3 release line 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

Comments

@hostep
Copy link
Contributor

hostep commented Jun 24, 2017

Preconditions

  1. Magento 2.2.0-RC1.1
  2. PHP 7.0.20

Steps to reproduce

  1. Execute
git clone -b 2.2.0-RC1.1 --single-branch --depth 1 https://github.com/magento/magento2.git ./magento22-test
cd ./magento22-test
composer install --optimize-autoloader
php bin/magento setup:install \
    --backend-frontname=secretadmin \
    --session-save=files \
    --db-host=localhost \
    --db-name=magento22-test \
    --db-user=root \
    --db-password=secret \
    --base-url=https://magento22-test.something/ \
    --language=en_US \
    --timezone=Europe/Brussels \
    --currency=EUR \
    --use-rewrites=1 \
    --use-secure=1 \
    --use-secure-admin=1 \
    --admin-use-security-key=1 \
    --admin-user=secretadmin \
    --admin-password=secret \
    --admin-email=email@example.com \
    --admin-firstname=Firstname \
    --admin-lastname=Lastname \
    --cleanup-database
  1. Now install the cronjobs and look at them:
bin/magento cron:install
crontab -l

Expected result

  1. Output
#~ MAGENTO START
* * * * * /opt/local/bin/php70 /path/to/magento22-test/bin/magento cron:run 2>&1 | grep -v "Ran jobs by schedule" >> /path/to/magento22-test/var/log/magento.cron.log
#~ MAGENTO END

Actual result

  1. Output:
#~ MAGENTO START
* * * * * /opt/local/bin/php70 /path/to/magento22-test/bin/magento cron:run 2>&1 | grep -v Ran jobs by schedule >> /path/to/magento22-test/var/log/magento.cron.log
* * * * * /opt/local/bin/php70 /path/to/magento22-test/update/cron.php >> /path/to/magento22-test/var/log/update.cron.log
* * * * * /opt/local/bin/php70 /path/to/magento22-test/bin/magento setup:cron:run >> /path/to/magento22-test/var/log/setup.cron.log
#~ MAGENTO END

Discussion

There are a couple of issues here:

  1. On the first line, the grep command should contain quotes, but they aren't there, resulting in errors
  2. The second line, there is no "update" directory, so why is this line added?
  3. The third line doesn't work properly, the setup.cron.log file contains this error:
[2017-06-24 08:31:45] setup-cron.ERROR: Could not locate magento/magento2-base/composer.json file. [] []

Other then that, I know Magento has been advocating to always setup those 3 command, but I never understood why. We've been only using the first line, never the other two. I don't even know what they are supposed to do? There is very little documentation around those. I "suspect" (but haven't verified), that they runs some composer update commands in the background, so all installed modules are kept up to date. But this is a terrible idea from our point of view, as you are supposed to test updates before rolling them out in production.
If my assumption of those two last commands are correct, then I'd appreciate it if an extra flag to the cron:install command is added, something like --i-know-what-im-doing which only creates the first line and doesn't setup the last 2 lines.

Feel free to inform me (and maybe update the devdocs at the same time) with some extra information about those last two commands, maybe I'm mistaken and maybe they are actually useful? :)

Thanks!

@fooman
Copy link
Contributor

fooman commented Jun 24, 2017

@hostep I am pretty sure that update/cron.php and setup:cron:run are used by the Web Setup Wizard, hence you probably not missing them.

@hostep
Copy link
Contributor Author

hostep commented Jun 24, 2017

Ok thanks for the extra info. That was kind of what I was suspecting.
In that case, for people not using the Web Setup Wizard, those two commands shouldn't get added to the crontab, right? So there should be some kind of option for the cron:install command to skip adding those two extra lines I believe.

@tjwiebell
Copy link
Contributor

Confirmed quote escaping bug in crontab creation, and created internal issue MAGETWO-70314 to resolve.

As @fooman mentioned, the other two commands are used by the Web Setup Wizard, and may be causing confusion for contributing developers, but should not be causing any harm. I'll bring this up at the next architecture meeting, and we'll create a feature request to separate those commands if there is enough support.

@misha-kotov misha-kotov added 2.2.0-rc1 Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Component: Config labels Jun 28, 2017
@hostep
Copy link
Contributor Author

hostep commented Jun 28, 2017

Thanks @tjwiebell!

Just to be clear: this isn't only a problem when you clone the git repo, the update directory is also missing when you install Magento using composer and run the bin/magento setup:install command to setup Magento. I assume this update directory gets created when using the Web Setup Wizard? But I've never once used that before, so I don't know for certain. I like to automate as much as possible, so I setup Magento using the command line. (this is based on Magento 2.1 experience, I don't think we can install 2.2 using composer yet at this time, so it's hard to tell if this will also be the case in 2.2)

Also since this cron:install command fits perfectly in this "trying to automate as much as possible", it makes sense to have some kind of flag to only setup the first line in the crontab, I don't think a lot of people who are using the Web Setup Wizard will actually use the cron:install command to be honest.

@tjwiebell
Copy link
Contributor

@hostep

The update directory comes from the magento/project-community-edition base project, and is created when running composer create-project .... All composer based and bundled archive installations will have this directory, regardless of how you installed Magento (Wizard or CLI).

I will review with the architect of this functionality, and will let you know if there's agreement to exclude these commands from git-based installations.

@hostep
Copy link
Contributor Author

hostep commented Jun 28, 2017

Thanks for the clarification. I double checked and you are correct!

But, we still don't have the update directory in any of our Magento 2 projects. Let me explain what we do:

  1. When we setup a project, we run composer create-project
  2. We have a .gitignore file where the update directory is set to being ignored, the idea was that the update directory was being installed by composer, so it shouldn't be in our projects git repo
  3. We put all the necessary files in our projects git repo and push to a remote
  4. Another developer clones the projects git repo and starts working on it, by first running composer install, he doesn't get an update directory, because composer install doesn't create it and because it isn't included in our git repo
  5. We do the same when deploying to a staging/production server, we clone the projects git repo, run composer install, and there is no update directory.

So this introduces a new question: should we put the update directory in our projects git repo?
If it is only being used for the Web Setup Wizard, and we never use that, I think the answer should be 'no' in our case, right?

Hope this sheds some more light into how we (and possibly others?) work with Magento 2 projects in the real world :)
But maybe we are doing it wrong?

Thanks!

@tjwiebell
Copy link
Contributor

Yes, that directory should be checked into source control. Not only are we releasing awesome new features in 2.2, but we've added some new articles that outline a suggested development pipeline:

http://devdocs.magento.com/guides/v2.2/config-guide/deployment/

I have gotten approval to create a feature request to fix the extraneous crontab commands in git-based installations. The proposed solution would also apply to your current development pipeline, and commands would be skipped if file/directory dependencies are not present, also displaying a message to the user that these commands were skipped. It likely won't make it into 2.2, but it will be addressed in a future release.

Thank you for submitting this feedback, we will let you know if we have any additional questions.

magento-team pushed a commit that referenced this issue Jul 6, 2017
Fixed issues:

MAGETWO-59997 Shipping address duplicates if entered credit card was incorrect during checkout via Braintree
MAGETWO-70279 Issue with the config merging introduced for the Analytics integration
MAGETWO-70059 Order is not shown in customer account if its Status not visible on Storefront
MAGETWO-59801 [Performance Bug] Tax Rules Form unusable with large # of tax rates
MAGETWO-70280 Catalog Event can't save in the en_GB locale
MAGETWO-70314 [Github] cron:install issues #10040
MAGETWO-69750 An error occurs on the checkout after required custom address attribute added
MAGETWO-70490 Inconsistency between versions in module.xml and UbgradeSchema for CatalogRule Module
MAGETWO-66895 [Github] Cannot skip review page when order placed with Virtual/Gift Card product via PayPal Express Checkout #9042
@ghost
Copy link

ghost commented Aug 16, 2017

@tjwiebell so what is the consensus on why the update directory is missing? I've done some create projects and sometimes it is missing, other times it is not. And running composer install + update without altering composer.json does not retrieve it.

I have also diff'ed composer.json files from an install with it and one without and they are the same. Is the update/ directory auto-generated somehow ?

@hostep
Copy link
Contributor Author

hostep commented Aug 16, 2017

@sylink: If I remember correctly, the update directory gets created when you perform a composer create-project, but then you are responsible for keeping it in your version control yourself since a composer install on another environment isn't going to create it again.

We currently don't keep that update directory in source control, since we don't need the Web Setup Wizard in our workflow, but everyone should decide for themselves if they want to keep the update directory in version control or not.

@magento-engcom-team magento-engcom-team added 2.2.0-rc1 Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Component: Config Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed and removed G1 Passed labels Sep 5, 2017
@okorshenko okorshenko self-assigned this Nov 1, 2017
@magento-engcom-team
Copy link
Contributor

Hello @hostep
Thank you for your report. The updater application + some CLI commands are designed for composer based installations only by the nature of these commands. So we do not expect they these feature will work on Git based installations. We are working right now to unify the dev experience for both installations. But this will lead to reorganization of our GitHub repository.
We are closing this as non-issue but we are working to improve this experience

@magento-engcom-team
Copy link
Contributor

Hi @hostep. Thank you for your report.
The issue has been fixed in #24187 by @atwixfirster in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming 2.3.4 release.

@magento-engcom-team magento-engcom-team added the Fixed in 2.3.x The issue has been fixed in 2.3 release line label Aug 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Component: Config Fixed in 2.3.x The issue has been fixed in 2.3 release line 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
Projects
None yet
Development

No branches or pull requests

6 participants