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

Avoid run of Request handle() and use of container at compiler pass to fix issues 3091 and 3553 #3558

Merged

Conversation

citlacom
Copy link
Contributor

Hi @jmolivas - @enzolutions, at Bluespark we have a project based on Drupal 8.3.7 and Drupal Console 1.0.0-beta4 that tried to upgrade to Drupal Console 1.0.2 but this caused a lot of fatal errors in our custom implementations of commands. I researched a bit of the existing issues and found that our error was very similar than #3091 so tried the workaround detailed on that issue to avoid usage of session but we found another fatal error that I reported at #3553

I tried to compare what have changed between the two versions to identify why this erros was not experimented in previous Drupal Console version and found that the main reason is the handle of the "/" default request which trigger hooks and globally display blocks access visibility rules among other things.

I reviewed all the Bootstrap process and started to experiment until have a bootstrap which don't caused those fatal exceptions. The main changes are:

  • Avoid handle() of the request and only process the preHandle().
  • Avoid to process build container on rebuildContainer and boot() to have only the initializeContainer on boot().
  • Avoid use of container on compiler pass because that causes PHP warnings and in some cases also errors and just discovered that based on Support for constructing parent service definition in build time symfony/symfony#22125 (comment) is recommended that container is not used at build phase, that error is exactly what I was experimenting in this project.

I have detailed with comments the reasons of all the changes so you can review, I was not really familiar with Drupal Kernel and Bootstrap process until this issue so I leave it to your expertise to decide what changes to preserve. I have tested multiples core and custom commands that are working properly. I would like to run tests for existing commands but couldn't found in documentation how to run tests in local. Can you guide me?

I hope this helps to guys that have been experimenting this weird problems when upgrading to last stable release and thanks for all your great job on the Drupal Console. I love it!

…xts of global displayed blocks when a general request is processed at Bootstrap/DrupalKernel. Also fixed major exceptions caused by AddServicesCompilerPass compiler pass class because is using container services when container is still on build phase, here is reduced the use of container but still there is code to refactor.
…at was not loaded and many commands as: 'user:login:url, site:status or cache:rebuild' was failing.
@jmolivas
Copy link
Member

@citlacom Thanks for the PR and the great explanation for the changes.

I tested the code and make some updates I will keep testing my changes and then merge this PR and apply my fixes.

@jmolivas jmolivas merged commit 3c4395b into hechoendrupal:master Nov 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants