From 8d087ae738d80c7de86c932726c0f388b538c4da Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 23 Jan 2015 13:31:45 -0800 Subject: [PATCH] Remove 'initFromConduit' option from Diffusion Summary: Ref T2783. I think this served two purposes: - Improving performance in cases where we "know" a repository is local. - Preventing loops. It is now obsolete: - After D11476, refs can almost always resolve on a fast path. - As T2783 moves forward, we can usually no longer know when a repository is local without actually looking it up -- almost everything is allowed to run anywhere. - The cluster behavior in D11475 now prevents loops. Test Plan: `grep`, browsed around. This didn't really do much of anything anymore. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T2783 Differential Revision: https://secure.phabricator.com/D11477 --- .../diffusion/DiffusionLintSaveRunner.php | 1 - .../DiffusionQueryConduitAPIMethod.php | 1 - .../diffusion/request/DiffusionRequest.php | 25 ++++++------------- ...habricatorRepositoryCommitHeraldWorker.php | 1 - .../PhabricatorOwnersPackagePathValidator.php | 1 - ...torRepositoryCommitMessageParserWorker.php | 1 - 6 files changed, 7 insertions(+), 23 deletions(-) diff --git a/src/applications/diffusion/DiffusionLintSaveRunner.php b/src/applications/diffusion/DiffusionLintSaveRunner.php index 6adcde8dda..d1f0005c21 100644 --- a/src/applications/diffusion/DiffusionLintSaveRunner.php +++ b/src/applications/diffusion/DiffusionLintSaveRunner.php @@ -236,7 +236,6 @@ final class DiffusionLintSaveRunner { foreach ($this->blame as $path => $lines) { $drequest = DiffusionRequest::newFromDictionary(array( 'user' => PhabricatorUser::getOmnipotentUser(), - 'initFromConduit' => false, 'repository' => $repository, 'branch' => $this->branch->getName(), 'path' => $path, diff --git a/src/applications/diffusion/conduit/DiffusionQueryConduitAPIMethod.php b/src/applications/diffusion/conduit/DiffusionQueryConduitAPIMethod.php index ad4f3a07f3..dd58cd09ab 100644 --- a/src/applications/diffusion/conduit/DiffusionQueryConduitAPIMethod.php +++ b/src/applications/diffusion/conduit/DiffusionQueryConduitAPIMethod.php @@ -102,7 +102,6 @@ abstract class DiffusionQueryConduitAPIMethod 'branch' => $request->getValue('branch'), 'path' => $request->getValue('path'), 'commit' => $request->getValue('commit'), - 'initFromConduit' => false, )); // Figure out whether we're going to handle this request on this device, diff --git a/src/applications/diffusion/request/DiffusionRequest.php b/src/applications/diffusion/request/DiffusionRequest.php index 29d8c3d5ec..9e804a54b5 100644 --- a/src/applications/diffusion/request/DiffusionRequest.php +++ b/src/applications/diffusion/request/DiffusionRequest.php @@ -215,10 +215,6 @@ abstract class DiffusionRequest { $this->didInitialize(); } - final protected function shouldInitFromConduit() { - return $this->initFromConduit; - } - final public function setUser(PhabricatorUser $user) { $this->user = $user; return $this; @@ -767,20 +763,13 @@ abstract class DiffusionRequest { // If we couldn't pull everything out of the cache, execute the underlying // VCS operation. if ($refs) { - if ($this->shouldInitFromConduit()) { - $vcs_results = DiffusionQuery::callConduitWithDiffusionRequest( - $this->getUser(), - $this, - 'diffusion.resolverefs', - array( - 'refs' => $refs, - )); - } else { - $vcs_results = id(new DiffusionLowLevelResolveRefsQuery()) - ->setRepository($this->getRepository()) - ->withRefs($refs) - ->execute(); - } + $vcs_results = DiffusionQuery::callConduitWithDiffusionRequest( + $this->getUser(), + $this, + 'diffusion.resolverefs', + array( + 'refs' => $refs, + )); } else { $vcs_results = array(); } diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php index e0aa650839..b1361c4b4a 100644 --- a/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php +++ b/src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php @@ -100,7 +100,6 @@ final class PhabricatorRepositoryCommitHeraldWorker $drequest = DiffusionRequest::newFromDictionary( array( 'user' => PhabricatorUser::getOmnipotentUser(), - 'initFromConduit' => false, 'repository' => $repository, 'commit' => $commit->getCommitIdentifier(), )); diff --git a/src/applications/repository/worker/commitchangeparser/PhabricatorOwnersPackagePathValidator.php b/src/applications/repository/worker/commitchangeparser/PhabricatorOwnersPackagePathValidator.php index c87e4145f5..d007b0b206 100644 --- a/src/applications/repository/worker/commitchangeparser/PhabricatorOwnersPackagePathValidator.php +++ b/src/applications/repository/worker/commitchangeparser/PhabricatorOwnersPackagePathValidator.php @@ -87,7 +87,6 @@ final class PhabricatorOwnersPackagePathValidator { id(new PhabricatorRepository())->load($commit->getRepositoryID()); $data = array( 'user' => PhabricatorUser::getOmnipotentUser(), - 'initFromConduit' => false, 'repository' => $repository, 'commit' => $commit->getCommitIdentifier(), ); diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php index 38a7f5c26b..192b1000e2 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php @@ -375,7 +375,6 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker } $drequest = DiffusionRequest::newFromDictionary(array( 'user' => PhabricatorUser::getOmnipotentUser(), - 'initFromConduit' => false, 'repository' => $this->repository, 'commit' => $this->commit->getCommitIdentifier(), 'path' => $path,