This is an archive of the discontinued Mercurial Phabricator instance.

phabricator: switch to urlgrabber for phabricator communications
ClosedPublic

Authored by mjpieters on Sep 22 2017, 12:56 PM.
Tags
None
Subscribers

Details

Summary

http://urlgrabber.baseurl.org/ is available on both corp and prod, without
additional dependencies specified, and a simple API.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mjpieters created this revision.Sep 22 2017, 12:56 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 22 2017, 12:56 PM
mjpieters retitled this revision from Switch to urlgrabber for phabricator communications to phabricator: switch to urlgrabber for phabricator communications.Sep 22 2017, 1:42 PM
mjpieters updated this revision to Diff 2040.
quark added a subscriber: quark.Sep 22 2017, 2:01 PM

Does this extension have tests? If so, we probably want to add python -c 'import ...' || exit 80 to skip the test if the library does not exist.

Besides, since this is a foreign extension. I guess you may need to change tests/modcheck.py.

In D799#13344, @quark wrote:

Does this extension have tests? If so, we probably want to add python -c 'import ...' || exit 80 to skip the test if the library does not exist.

Yes, but they mock out all code that calls urlgrabber. We should probably change that to mocking just that module; the current setup won't let us test D800 for example.

Besides, since this is a foreign extension. I guess you may need to change tests/modcheck.py.

I'll take a look.

quark accepted this revision.Sep 25 2017, 9:18 PM
In D799#13344, @quark wrote:

Does this extension have tests? If so, we probably want to add python -c 'import ...' || exit 80 to skip the test if the library does not exist.

Yes, but they mock out all code that calls urlgrabber. We should probably change that to mocking just that module; the current setup won't let us test D800 for example.

That sounds good enough. I think it's fine as-is. Improving testing seems a lot of work without much benefit in this case.

Besides, since this is a foreign extension. I guess you may need to change tests/modcheck.py.

I'll take a look.

You can use arc unit which will load modcheck.py to see if it causes problems. If all external code gets wrapped, in theory it's good because the external library won't get a chance to be imported.

This revision is now accepted and ready to land.Sep 25 2017, 9:18 PM
This revision was automatically updated to reflect the committed changes.

This breaks hg ssl on Windows. We can't just install dependencies there, we need to bundle them.