Unexpected output: Affected paths: relative - resubmit patch.
⚙ D50 INP-1413 - Don't do full Application Init during deployment
Page MenuHomeIn-Portal Phabricator

INP-1413 - Don't do full Application Init during deployment
ClosedPublic

Authored by alex on Mar 19 2015, 9:18 AM.

Details

Test Plan
NOTE: Use PHP 5.4+ during this test

Preparations:

  1. don't apply patch yet
  2. open the ToolsSystem Tools section in Admin Console in one tab
  3. open homepage in other tab and confirm, that no error happens
  4. create an empty trait (in separate file) in the /core/units/users/ folder
  5. register trait in RegisterClasses array in users_config.php (don't rebuild any caches yet)
  6. use trait in the UsersEventHandler class
  7. reload tab with homepage and confirm, that now Fatal Error (related to missing trait) is shown in debugger

Actual test:

  1. press Deploy button in ToolsSystem Tools section in Admin Console and confirm, that now Fatal Error (related to missing trait) is shown in debugger
  2. run do_update script on In-Portal folder and confirm that it ends up with same Fatal Error
  3. apply patch
  4. run do_update script and confirm, that no error happens and homepage can be opened after that
  5. rewind the test to get fatal error again
  6. press Deploy button in ToolsSystem Tools section in Admin Console and confirm, that no error happens and homepage can be opened after that

Diff Detail

Repository
rINP In-Portal
Branch
/in-portal/branches/5.2.x
Lint
Lint OK
Unit
No Unit Test Coverage

Event Timeline

alex updated this revision to Diff 125.Mar 19 2015, 9:18 AM
alex retitled this revision from to INP-1413 - Don't do full Application Init during deployment.
alex updated this object.
alex edited the test plan for this revision. (Show Details)
alex added 1 JIRA issue(s): INP-1413.
glebs accepted this revision.Mar 25 2015, 3:55 AM
glebs edited edge metadata.
This revision is now accepted and ready to land.Mar 25 2015, 3:55 AM
alex planned changes to this revision.Apr 27 2015, 4:51 AM

I've looked deeply at what the adm:OnStartup even really is and what hooks to it do and it can be summarized into: create cookies/session variables based on request/other cookies.

No request or cookies exists in CLI, so we can automatically skip it in there.

core/kernel/application.php
384–386

Use PHP_SAPI !== 'cli' instead of 1 to prevent adm:OnStartup event from executing while in command line.

tools/run_event.php
39–42 ↗(On Diff #125)

Delete.

alex added inline comments.Apr 27 2015, 6:17 AM
core/kernel/application.php
384–386

In fact we can replace that IF with following and remove all other changes in this diff:

if ( PHP_SAPI !== 'cli' && !$this->isAdmin ) {
alex updated this revision to Diff 166.Apr 27 2015, 3:05 PM
alex edited edge metadata.

Run the adm:OnStartup event only, when Front-End is used from browser

alex added a comment.Apr 29 2015, 2:33 AM

After successful testing please also:

  1. revert rATV16895 and rATV16896
  2. apply this new patch there
  3. test that all works (e.g. adding new trait to EUserEventHandler won't break deploy on another machine
glebs accepted this revision.Apr 29 2015, 12:57 PM
glebs edited edge metadata.
This revision is now accepted and ready to land.Apr 29 2015, 12:57 PM
This revision was automatically updated to reflect the committed changes.
alex edited edge metadata.Mar 10 2016, 6:47 AM
alex added a project: Restricted Project.