Page MenuHomePhabricator

Use Conduit in PhabricatorRepositoryGitCommitChangeParserWorker
ClosedPublic

Authored by epriestley on Feb 24 2015, 2:59 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 27, 2:34 AM
Unknown Object (File)
Wed, Mar 27, 2:34 AM
Unknown Object (File)
Wed, Mar 27, 2:34 AM
Unknown Object (File)
Sun, Mar 24, 12:14 AM
Unknown Object (File)
Sat, Mar 23, 9:32 PM
Unknown Object (File)
Sun, Mar 17, 1:56 PM
Unknown Object (File)
Fri, Mar 15, 12:22 PM
Unknown Object (File)
Sun, Mar 10, 12:35 AM
Tokens
"Mountain of Wealth" token, awarded by joshuaspence.

Details

Summary

Ref T2783. This allows this worker to run on a machine different to the one that stores the repository, by routing the execution of Git over Conduit calls.

This API method is super gross, but fixing it isn't straightforward and it runs into other complicated considerations. We can fix it later; for now, just define it as "internal" to limit how much mess this creates.

"Internal" methods do not appear on the console.

Test Plan

Ran bin/repository reparse --change <commit> --trace on several commits, saw daemons make a Conduit call instead of running a git command.

Diff Detail

Repository
rP Phabricator
Branch
havail1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 11605
Build 14510: Run Core Tests
Build 14509: arc lint + arc unit

Event Timeline

hach-que retitled this revision from to Use Conduit instead of running `git log` directly in PhabricatorRepositoryGitCommitChangeParserWorker.
hach-que updated this object.
hach-que edited the test plan for this revision. (Show Details)
hach-que added a reviewer: epriestley.
epriestley edited edge metadata.
epriestley added inline comments.
src/applications/diffusion/conduit/DiffusionRawDiffStatQueryConduitAPIMethod.php
3 ↗(On Diff #28596)

This should only support Git. I think it will try to run git commands in hg/svn repositories right now? Some of the other API methods should have example code for dealing with this.

11 ↗(On Diff #28596)

pht()

src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryGitCommitChangeParserWorker.php
22–25

Omit this; all pathways which fail to produce output throw an exception so this code is unreachable.

This revision now requires changes to proceed.Feb 27 2015, 7:42 PM
hach-que edited edge metadata.

Changes requested in code review.

Tested again via the Conduit web API (did not run daemons / workers).

hach-que retitled this revision from Use Conduit instead of running `git log` directly in PhabricatorRepositoryGitCommitChangeParserWorker to [ha/daemon-tier] Use Conduit instead of running `git log` directly in PhabricatorRepositoryGitCommitChangeParserWorker.Jul 1 2015, 5:00 AM
hach-que edited edge metadata.

@epriestley Is this still relevant, or has it been obsoleted upstream?

@epriestley Can you commandeer this? It's sitting in my queue and there's nothing left for me to do with it.

Otherwise I'll just abandon it given it's been sitting here for like 8 months.

I'd really like to see this upstreamed.

Abandoning this because I don't have time to maintain it. @epriestley feel free to pick this up again once you're scaling daemons in the Phacility cluster.

epriestley edited reviewers, added: hach-que; removed: epriestley.

I wasn't really happy with just moving this mess into a method wholesale, but teasing it apart seems unlikely to be worthwhile in service of T2783.

epriestley retitled this revision from [ha/daemon-tier] Use Conduit instead of running `git log` directly in PhabricatorRepositoryGitCommitChangeParserWorker to Use Conduit in PhabricatorRepositoryGitCommitChangeParserWorker.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
  • Update message and so on.
chad edited edge metadata.
This revision is now accepted and ready to land.Apr 9 2016, 6:22 PM
This revision was automatically updated to reflect the committed changes.