Page MenuHomePhabricator

Use Conduit in PhabricatorRepositoryGitCommitChangeParserWorker
ClosedPublic

Authored by epriestley on Feb 24 2015, 2:59 AM.
Tags
None
Referenced Files
F11033000: D11874.id37743.diff
Sun, Aug 14, 11:56 AM
Unknown Object (File)
Sun, Jul 31, 7:00 PM
Unknown Object (File)
Sun, Jul 31, 1:58 PM
Unknown Object (File)
Sat, Jul 30, 6:50 AM
Unknown Object (File)
Jul 15 2022, 12:39 AM
Unknown Object (File)
Jul 12 2022, 12:55 PM
Unknown Object (File)
Jul 9 2022, 12:13 AM
Unknown Object (File)
Jun 25 2022, 7:06 PM
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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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.