Commit Graph

18 Commits

Author SHA1 Message Date
Adam Simpkins
30d1d145d8 conduit: cache and re-use conduit connections
Summary:
Update the phabricator/conduit.py module to cache connections to Phabricator
and re-use them, rather than creating a brand new connection each time
call_conduit() is invoked.

This makes the `{phabstatus}` template reasonable to use outside of the
smartlog extension.  There was already custom logic for smartlog so that it
only makes a single conduit call to look up data on all revisions at once.
However, generic `log` commands would still end up creating a separate
connection to phabricator for each revision.  With this change `log` still ends
up making one call per revision, but at least it does not set up and tear down
a separate connection for each one any more.

Test Plan:
Tested using the `{phabstatus}` template with `hg log` and confirmed it no
longer made a separate connection to phabrictor for each revision.  It is still
pretty slow, but no longer quite as bad.

Reviewers: #mercurial, simonfar, tomaszo

Reviewed By: simonfar

Subscribers: simonfar, medson, mjpieters, net-systems-diffs@fb.com

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

Signature: t1:5383425:1499680847:428185ba762fefbe12a411a1afa0a090aafda2f2
2017-07-12 20:00:00 -07:00
Mark Edson
dd3fd72496 phabricator: override default ph*.fb.com with ph*.intern.facebook.com
Summary:
Trivial change.  Rather than run around and change all 200+ repos so their .arcconfigs are pointing to phabricator.intern.facebook.com, just override in hgext.  The hgext call to differential.querydiffhashes here:
  https://phabricator.intern.facebook.com/diffusion/HGEXT/browse/default/hgext3rd/phabstatus.py$76
represents 20k hits per day to phabricator.fb.com, and that is now (by far) the biggest hitter of fb.com, so we'd like it to go away.

Test Plan:
Modified my .arcconfig and .arcrc to both point to phabricator.fb.com and verified with temporary logging that the redirect was happening.

Because we already added the code to make it hunt down the first cert it finds, this doesn't hurt the connection between URL and certificate in .arcrc

Reviewers: mitrandir, rmcelroy, #phabricator, #sourcecontrol, simonfar

Reviewed By: simonfar

Subscribers: medson, mjpieters

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

Tasks: 17358638

Signature: t1:5320052:1498488854:27f56d98f8dcb9cf5a6900360316a99d42e353c5
2017-06-26 08:04:42 -07:00
Durham Goode
e34660b057 commands: update to use registrar instead of cmdutil
Summary: Upstream has deprecated cmdutil.commands() in favor of registrar.commands()

Test Plan: Ran the tests

Reviewers: #mercurial, quark

Reviewed By: quark

Subscribers: mjpieters

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

Signature: t1:5106486:1495485074:0e20f00622cc651e8c9dda837f84dd84cc51099e
2017-05-22 13:38:37 -07:00
Misha Shneerson
cbdaa23f88 conduit: added correct Content-Type header for conduit queries
Summary:
conduit HTTP POST request did not have correct content-type header set.
Now they do. Ideally we should use python-request package to do HTTP
but I do not know enough how to import these dependencies so I just went
with the minimal changes.

Test Plan:
ran hg log from dev environment and eventually got it all working.
{P57411895}

Reviewers: simpkins

Reviewed By: simpkins

Subscribers: net-systems-diffs@fb.com, mjpieters

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

Tasks: 18294945

Signature: t1:5080725:1495052201:1bbd6edad4c1c4b3482a7479259460a815947630
2017-05-17 13:50:37 -07:00
Mark Edson
36d60f5259 phabricator: make conduit auth more flexible
Summary:
We've modified arcanist a while ago so that if it doesn't find user/cert information for the specific URL that it's using, it just picks the user/cert information from any other URL in ~/.arcrc.  This is because it's essentially always the same user, and always the same cert, so there's really no point in being too picky.
This updates hg extension to be almost as careless.  It will attempt to find the matching user/cert, but if it doesn't work, it'll just pick any cert if available.

Test Plan:
Without this change, "hg ssl" in a recent version of www reports an warning because www/.arcconfig has phabricator.intern.facebook.com, where default ~/.arcrc's have phabricator.fb.com.
With this change, "hg ssl" succeeds to display revision information because it is once again able to authenticate with conduit

Reviewers: #phabricator, rmcelroy

Reviewed By: rmcelroy

Subscribers: mjpieters

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

Tasks: 17683952

Signature: t1:4964111:1493323636:668b50ce2d20d720ba3de573de05be5251ce3310
2017-04-27 13:45:50 -07:00
Kostia Balytskyi
b331423be1 phabricator: look for .arcrc in the correct location on Windows
Summary:
On Windows `arc install-certificate` writes the cert to APPDATA:
https://fburl.com/i7fpssf8, so that is where we need to look for it.

Test Plan: - make this change, see that `hg ssl` shows commit status locally

Reviewers: davidsp, andrasbelo, #sourcecontrol

Subscribers: mjpieters

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

Tasks: 17511545
2017-04-27 09:01:15 -07:00
Adam Simpkins
36dcf86559 phabstatus: fail gracefully if necessary arcrc settings are missing
Summary:
If the user does not have necesary credentials defined in their arc
configuration, catch the KeyError and convert it into an ArcConfigError.

The existing call sites in the phabstatus and arcdiff extensions catch and
handle ArcConfigError, but not generic KeyErrors.

This also fixes the phabstatus warning messages to end with a newline.

Test Plan: Added a unit test.

Reviewers: #sourcecontrol, quark, simonfar, wez, rmcelroy

Reviewed By: wez, rmcelroy

Subscribers: rmcelroy, net-systems-diffs@fb.com, yogeshwer, mjpieters

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

Tasks: 17002914

Signature: t1:4800977:1490847078:e18bba042e3ff57100e0a7b25c610b5cad17fa2e
2017-03-30 11:55:39 -07:00
Adam Simpkins
675d0280a6 phabdiff: only match at the start of a line
Summary:
Only match the "Differential Revision" label at the start of a line.  We
have some diffs that include legitimate-looking Differential Revision label
strings inside parts of their test plan, which previously confused the phabdiff
output.

Test Plan: Included a unit test.

Reviewers: #sourcecontrol, quark, akushner

Reviewed By: akushner

Subscribers: net-systems-diffs@fb.com, yogeshwer, mjpieters

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

Signature: t1:4793483:1490756514:05b2c49d5d65e1a3eadd5eb78ed0b91ef3bea42c
2017-03-28 20:53:07 -07:00
Simon Farnsworth
ed9d987450 phabricator: switch from urlparse to urlreq for URI parsing
Summary:
This follows the removal of urlparse upstream:

changeset:   97aa316e17ea7a0d3ba4180b23ef29bd32e409fc
user:        Gregory Szorc <gregory.szorc@gmail.com>
date:        Tue, 21 Mar 2017 22:47:49 -0700

    py3: stop exporting urlparse from pycompat and util (API)

Test Plan:
./run-tests.py locally

Copy extension into place on my devserver and confirm that phrevset still works in www:
04:27:55 simonfar@devvm022:~/www (bb5bd09) $ hg show D1234
D1234 does not have an associated version control system
You can view the diff at http://phabricator.fb.com/D1234

abort: unknown revision D1234

04:28:53 simonfar@devvm022:~/www (bb5bd09) $ hg show D4721979
changeset:   74f087400ed442941d7400749d7f0301f7b6662a  D4721979
user:        alonsch@2c7ba8d8-a2f7-0310-a573-de162e16dcc7
date:        Mon, 20 Mar 2017 04:07:37 -0700

    [Portal] Remove outdated TODO comments in OnavoLineChart

Reviewers: #sourcecontrol, ikostia

Subscribers: mjpieters

Differential Revision: https://phabricator.intern.facebook.com/D4761905
2017-03-23 06:02:24 -07:00
Stanislau Hlebik
dd1bfa42b8 phabdiff: precompile regexes
Summary:
I got the following profile results when profiling smartlog:

{P56936293}

It suggests that regex compiling takes too much time. I tend to think that
profiling is inaccurate in this case because with this diff I didn't get any
noticable speed up. But this diff won't do any harm.

Test Plan: arc unit

Reviewers: #sourcecontrol, rmcelroy

Reviewed By: rmcelroy

Subscribers: rmcelroy, mjpieters

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

Signature: t1:4387265:1483724595:43de3c694109be2d4343d8ebdbc7ab79aa9edb04
2017-01-09 01:37:48 -08:00
Jun Wu
d203784d68 pyflakes: fix all pyflakes issues
Summary: As the title.

Test Plan: `arc unit`

Reviewers: #sourcecontrol, stash, rmcelroy

Reviewed By: stash, rmcelroy

Subscribers: rmcelroy, stash, mjpieters

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

Signature: t1:4232321:1480067588:54e91ece8fa6b5ff13b3ebc9279217c76bf96a24
2016-11-25 00:23:21 +00:00
Ryan McElroy
390c00a600 conduit: stop using httplib
Summary: This allows us to pass the check-code test.

Test Plan: run all tests

Reviewers: #mercurial, ttung, quark

Reviewed By: quark

Subscribers: mjpieters

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

Signature: t1:3514693:1467624953:99a15876c9eb78f79a05ad315f3eb9dbd0b70fb8
2016-07-04 03:00:37 -07:00
Martijn Pieters
4ba66d8405 phabricator: use new mercurial.util.urlparse reference
Summary:
Silence the test-check-code-hg test failure by using the new
mercurial.util.urlparse location (used to bridge Python 2 and 3).

Test Plan:
Ran all phabricator tests:

  $HG/tests/run-tests.py -l test-phab*

Reviewers: #mercurial, ttung, simonfar

Reviewed By: simonfar

Subscribers: simonfar, mjpieters

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

Signature: t1:3503125:1467283251:dd77c1dc7037d35c80c17ccf89d96c16168aa117
2016-06-30 11:41:45 +01:00
Wez Furlong
1aecce519b hgext: refactor last-diff code into diffprops.py
Summary:
I want to re-use this elsewhere, so robustify it a bit
and move it to phabricator.diffprops.

Test Plan: run-tests.py, also verified in my www repo.

Reviewers: #sourcecontrol, ttung

Reviewed By: ttung

Subscribers: mjpieters

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

Signature: t1:3230147:1463081787:799f232e2ce73395218db3a0fff37dec9a0b02e0
2016-05-12 13:11:08 -07:00
Wez Furlong
7972567aef phabdiff: make phabdiff more URL-agnostic
Summary: We want out phabricator diff parsers to recognize both https://phabricator.intern.facebook.com/ and https://phabricator.fb.com/ URLs (and a bigger class as well).

Test Plan:
- updated some tests (for `phabdiff` and `pullcreatemarkers` other files don't even seem to be used)
- ran tests
- tested phabdiff manually as well:
{F60696023}

Reviewers: #sourcecontrol, andersonmat, mitrandir, simpkins, lcharignon, quark, ttung, ikostia, rmcelroy

Reviewed By: ikostia, rmcelroy

Subscribers: wez, rmcelroy, net-systems-diffs@, mjpieters

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

Tasks: 11013909, 11017978

Signature: t1:3229776:1461839346:08b9b3414e43ff9966bc05591ca5662ef9691aac
2016-04-28 10:55:06 -07:00
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
Wez Furlong
12a08e0a47 add arcconfig accessors
Summary:
This is here to support other arcanist/phabrication integration
modules.

Note: this adds a new package.  I'm not sure what the packaging ramifications
are exactly, but surely there are some as other extensions start to depend on
this.

Test Plan:
integration test is provided:

```
$ ../../hg-crew/tests/run-tests.py -j8
....................................................
# Ran 52 tests, 0 skipped, 0 warned, 0 failed.
```

Reviewers: #sourcecontrol, ttung

Subscribers: mjpieters

Differential Revision: https://phabricator.fb.com/D3215615
2016-04-27 09:27:56 -07:00