Page MenuHomePhabricator
Differential D11874

Use Conduit in PhabricatorRepositoryGitCommitChangeParserWorker
ClosedPublic
Actions

Authored by epriestley on Feb 24 2015, 2:59 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 30, 12:28 AM
Unknown Object (File)
Sat, Dec 28, 4:52 PM
Unknown Object (File)
Fri, Dec 27, 3:14 AM
Unknown Object (File)
Tue, Dec 10, 4:10 PM
Unknown Object (File)
Nov 26 2024, 10:40 PM
Unknown Object (File)
Nov 18 2024, 7:08 AM
Unknown Object (File)
Nov 4 2024, 2:35 PM
Unknown Object (File)
Nov 4 2024, 2:20 PM
View All Files
Subscribers
epriestley
joshuaspence
Korvin
Tokens
"Mountain of Wealth" token, awarded by joshuaspence.

Details

Reviewers
hach-que
chad
Maniphest Tasks
T2783: Make working-copy operations service-oriented
Commits
rP7150aa8e192e: Use Conduit in PhabricatorRepositoryGitCommitChangeParserWorker
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 11604
Build 14508: Run Core Tests
Build 14507: arc lint + arc unit

Event Timeline

hach-que updated this revision to Diff 28596.Feb 24 2015, 2:59 AM
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.
hach-que added a task: T2783: Make working-copy operations service-oriented.
Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptFeb 24 2015, 2:59 AM
Herald added subscribers: epriestley, Korvin. · View Herald Transcript
joshuaspence added a subscriber: joshuaspence.Feb 25 2015, 4:49 AM
epriestley requested changes to this revision.Feb 27 2015, 7:42 PM
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 updated this revision to Diff 32274.Jun 18 2015, 2:19 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.
hach-que added a comment.Dec 3 2015, 9:44 AM

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

epriestley mentioned this in T4209: Multiserver / High-Availability Configuration.Dec 4 2015, 12:20 PM
hach-que mentioned this in T10029: PhabricatorRepositoryGitCommitChangeParserWorker is unable to find a commit.Dec 21 2015, 2:07 PM
hach-que added a comment.Dec 22 2015, 1:00 AM

@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.

joshuaspence added a comment.Dec 22 2015, 1:08 AM

I'd really like to see this upstreamed.

joshuaspence awarded a token.Dec 22 2015, 1:08 AM
hach-que updated this revision to Diff 35890.Dec 22 2015, 1:13 AM

Rebase on origin/master

hach-que abandoned this revision.Apr 4 2016, 2:13 AM

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 commandeered this revision.Apr 8 2016, 8:08 PM
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 updated this revision to Diff 37742.Apr 8 2016, 8:20 PM
  • Move to internal method.
epriestley updated this revision to Diff 37743.Apr 8 2016, 8:21 PM
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.
epriestley edited reviewers, added: chad; removed: hach-que, Blessed Reviewers.Apr 8 2016, 8:22 PM
epriestley added a reviewer: hach-que.
epriestley mentioned this in T2783: Make working-copy operations service-oriented.Apr 8 2016, 8:41 PM
chad accepted this revision.Apr 9 2016, 6:22 PM
chad edited edge metadata.
This revision is now accepted and ready to land.Apr 9 2016, 6:22 PM
epriestley mentioned this in T10751: Make Phabricator Highly Available.Apr 10 2016, 4:00 AM
Closed by commit rP7150aa8e192e: Use Conduit in PhabricatorRepositoryGitCommitChangeParserWorker (authored by hach-que, committed by epriestley). · Explain WhyApr 14 2016, 11:53 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
src/
2 lines
applications/
conduit/
method/
3 lines
query/
14 lines
1 line
diffusion/
conduit/
74 lines
repository/
worker/
commitchangeparser/
44 lines

Diff 37742

View Options

src/__phutil_library_map__.php

Loading...
View Options

src/applications/conduit/method/ConduitAPIMethod.php

Loading...
View Options

src/applications/conduit/query/PhabricatorConduitMethodQuery.php

Loading...
View Options

src/applications/conduit/query/PhabricatorConduitSearchEngine.php

Loading...
View Options

src/applications/diffusion/conduit/DiffusionInternalGitRawDiffQueryConduitAPIMethod.php

Loading...
View Options

src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryGitCommitChangeParserWorker.php

Loading...
Phabricator · Built by Phacility