Commit Graph

15 Commits

Author SHA1 Message Date
Wez Furlong
628f88d82d hgext: refactor D123 parsing, centralize
Summary:
In addition to being duplicated between these places,
I'd like to re-use this elsewhere.

Test Plan: run-tests continues to pass

Reviewers: #sourcecontrol, ttung, ikostia

Reviewed By: ikostia

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D3230128

Tasks: 11013909

Signature: t1:3230128:1461775513:6fc79fda68cd15ded7fb11d52024b5aab56ee880
2016-04-27 09:50:13 -07:00
Wez Furlong
1059b7253f Add conduit client
Summary:
I pretty much stole this from our libfb.py.conduit client, but
removed the python 2.7 and 3 specific aspects of it.

This is an HTTP client for conduit, rather than shelling out to arcanist.

I've added a very simple mechanism for replaying conduit results in the
test harness and used this to build out some tests for the `arcdiff.py`
and `phabstatus.py` extensions.

Test Plan:

```
$ ../../hg-crew/tests/run-tests.py -j8
```

In addition to the new tests, manually tested the actual HTTP functionality:

```
$ /data/users/wez/facebook-hg-rpms/hg-crew/hg --config extensions.phabstatus=/data/users/wez/facebook-hg-rpms/fb-hgext/phabstatus.py --config extensions.errorredirect=! ssl
```

Does not error out and shows the diff status.

Reviewers: #sourcecontrol, ttung

Subscribers: mjpieters

Differential Revision: https://phabricator.fb.com/D3200713
2016-04-27 09:27:56 -07:00
Adam Simpkins
6c3719ed46 phabstatus: clean up error handling
Summary:
Don't blindly swallow all exceptions in getdiffstatus().  Previously this was
masking bugs in the code.  Instead, only catch exceptions thrown by the initial
Popen() call.  Also check the the process return code, and report an error if
it exits unsuccessfully.  Fix a return statement that was incorrectly returning
an erorr message instead of replacement strings.

Also fix the code to use subprocess.communicate() instead of only reading from
proc.stdout.  The old behavior could result in a hang if arc tried to print
data on stderr but we weren't reading it.

Test Plan:
Manually tested the error code paths by forcing arc to fail locally on my dev
server.

Reviewers: #sourcecontrol, ttung, lcharignon

Reviewed By: lcharignon

Subscribers: net-systems-diffs@, yogeshwer, mjpieters

Differential Revision: https://phabricator.fb.com/D3146077

Signature: t1:3146077:1459971594:ebab6a0ccb7bea66da1e23dcb88633c48b3e952b
2016-04-06 12:49:43 -07:00
Adam Simpkins
eb7787dd48 unbreak the phabstatus extension
Summary:
D3022145 caused phabstatus to always fail, since it ended up passing the
timeout as to subprocess.Popen() as an integer instead of a string.

Test Plan:
Ran "hg log -r. -T'{phabstatus}\n'" and confirmed it displayed diff status
correctly, and no longer complained about not being able to run arc.

It looks like there currently aren't any existing unit tests for this
extension.

Reviewers: #sourcecontrol, andersonmat, lcharignon, ttung

Reviewed By: ttung

Subscribers: gkeramidas, net-systems-diffs@, yogeshwer, mjpieters

Differential Revision: https://phabricator.fb.com/D3145902

Signature: t1:3145902:1459968939:b0348cd20f216f7e7a2929812b24eae18e5d17c3
2016-04-06 12:14:09 -07:00
Adam Simpkins
81d088d2a8 phabstatus: fix extraneous semicolon
Summary: The test-check-code-hg.t test was failing due to this trailing semicolon.

Test Plan: Ran the test-check-code-hg.t, confirmed it no longer complains about the code.

Reviewers: #sourcecontrol, lcharignon, ttung, andersonmat

Reviewed By: andersonmat

Subscribers: net-systems-diffs@, yogeshwer, mjpieters

Differential Revision: https://phabricator.fb.com/D3080440

Signature: t1:3080440:1458622832:f327c89378d3c8aadee9e69e391ed6a5b08af915
2016-03-22 11:44:36 -07:00
Matt Anderson
5a3f2bd489 [ssl] add phabricator timeout option
Test Plan: tested timeout on local repo to ensure it aborted the request correctly

Reviewers: lcharignon

Reviewed By: lcharignon

Subscribers: ssl-diffs@, mjpieters, jpasqualini

Differential Revision: https://phabricator.fb.com/D3022145

Tasks: 10112339

Signature: t1:3022145:1458227284:8aa17f8e4f64e4b8cb2b04a55c3939f7bf2e011f
2016-03-17 08:09:36 -07:00
Laurent Charignon
81f91c3b39 phabstatus: don't query phabricator is smartlog has no revision with diff
Summary: See https://our.intern.facebook.com/intern/tasks/?t=10060014

Test Plan: Tests pass

Reviewers: #sourcecontrol, ttung, quark

Reviewed By: quark

Subscribers: quark, mjpieters, victorl

Differential Revision: https://phabricator.fb.com/D2938055

Tasks: 10060014

Signature: t1:2938055:1455617452:4fd48f23aa51c193d9717a17081810ca6a01b349
2016-02-23 09:24:58 -08:00
Laurent Charignon
35d3252f3f phabstatus: improve reliability when conduit is down or sending errors
Summary:
This patches makes "hg sl" and requesting diff status in general more
reliable. We display "Error" for the diff status and log the error in the
console.

    $ ~/facebook-hg-rpms/fb-hgext hg ssl
    Could not call "arc call-conduit" No JSON object could be decoded
    @  8844c9  lcharignon  D2824091  Error
    |  phabstatus: improve reliability when conduit is down or sending errors
    |
    o  252837  lcharignon  D2821846  Error  remote/@
    |  setup: remove writecg2 from list of module
    |
    .
    .
    |
    | o  9ad683  ericsumner  remote/stable
    | |  pushrebase: add HG_PENDING to test
    | |
    | o  e2b354  ericsumner
    |/   writecg2: update test, no longer writing CG2 bundles
    |
    o  fcd19f  durham
    |  Add dirsync to setup.py
    |
    note: hiding 7 old heads without bookmarks


Test Plan: Checked with wifi off that the message is showed

Reviewers: #sourcecontrol, ttung

Differential Revision: https://phabricator.fb.com/D2824091
2016-01-14 10:45:47 -08:00
Laurent Charignon
728e89ef80 phabstatus: make hg ssl very fast!
Summary:
For 10 diffs in fbsource

    hg ssl before the patch => 9s
    hg ssl after the patch => 1.5s
    hg sl => 1.2s

This patch is a little tricky, we:
- improve the method to get diff status by making it accept a list of diffnum
- wrap smartlog's getdag method to store the smartlogrevs
- add a hack around the memoization mechanism to memoize all individual calls
  for all the diffs when we make the call for all the diffs together (see code
  for details)
- resolve all the phabricator revisions requested by smartlog at once when we
  are asked to resolve the first one

Test Plan:
hg sl does not take longer and works
hg ssl is faster after the patch and produces the same output as before

Reviewers: durham, rmcelroy, cdelahousse

Subscribers: ssl-diffs@

Differential Revision: https://phabricator.fb.com/D2818828
2016-01-11 10:30:13 -08:00
Laurent Charignon
a42808982c cleanup: fix the "line too long" warnings
Summary: This diff is part of a series to cleanup fb-hgext and make it pass check-code.

Test Plan: all tests pass

Reviewers: #sourcecontrol, ttung

Differential Revision: https://phabricator.fb.com/D2811926
2016-01-07 18:30:24 -08:00
Laurent Charignon
53a55a42c1 cleanup: fix legacy exception syntax
Summary: This diff is part of a series to cleanup fb-hgext and make it pass check-code.

Test Plan: all tests pass

Reviewers: #sourcecontrol, ttung

Differential Revision: https://phabricator.fb.com/D2811871
2016-01-07 18:30:24 -08:00
Laurent Charignon
be7b9d9620 cleanup: remove gratuitous spaces
Summary: This diff is part of a series to cleanup fb-hgext and make it pass check-code.

Test Plan: all tests pass

Reviewers: #sourcecontrol, ttung

Differential Revision: https://phabricator.fb.com/D2811864
2016-01-07 18:30:24 -08:00
Christian Delahousse
fe8d11dede documentation: added phabdiff and phabstatus template names
Summary: template names were not showing up in the documentation

Test Plan:
  12/08 11:20 cdelahousse@dev4253 /data/users/cdelahousse/fb-hgext
  $ hg help template --config extensions.phabdiff=phabdiff.py --config extensions.phabstatus=phabstatus.py
  phabdiff      String. Return the phabricator diff id for a given hg rev
  phabstatus    String. Return the diff approval status for a given hg rev

Reviewers: #sourcecontrol, ttung

Differential Revision: https://phabricator.fb.com/D2735148

Tasks: 9287658
2015-12-08 11:28:38 -08:00
Rajesh Janakiraman
d1383285ff memoize phabstatus results
Summary: Part of the delay in the hg ssl call was because phabstatus was actually making the network call for each time it was called in the template condition, and we can cache the results

Test Plan: Ran hg ssl, so much faster

Reviewers: durham, rmcelroy

Differential Revision: https://phabricator.fb.com/D2652050
2015-11-17 15:13:12 +00:00
Rajesh Janakiraman
3cb6727ebe Added phabstatus to query phabricator for a diff status
Test Plan:
  rajeshj@rajeshj-mbp1:fbandroid  (checkup)$ hg log -r 45de9b --template '{phabstatus}'
  Needs Review

Reviewers: durham, rmcelroy

Differential Revision: https://phabricator.fb.com/D2632410
2015-11-09 21:24:13 +00:00