Works nicely, but here are two nit-picks:

* script should be named check-url-parallel or something similar so that users know why we have two scripts with almost same name but not quite
* --host-pro should probably be --host-intranet to better convey meaning of flag

You still love --host-pro flag I guess :-)

I needed to read source code twice to figure out which URL this is so I really think that flag name needs change.

I had a quick chat with Chris about dependencies on AnyEvent::HTTP which isn't part of Debian stable and came out with solution to mark both AnyEvent and AnyEvent::HTTP as optional dependency (using required => 0) so that we don't bring in additional modules but still offer then as suggested in Debian package.

> You still love --host-pro flag I guess :-)

Too bad! I've changed the help text, and forgot to change the code. I will do that this evening, quietly.
 
> I had a quick chat with Chris about dependencies on AnyEvent::HTTP which
> isn't part of Debian stable and came out with solution to mark both AnyEvent
> and AnyEvent::HTTP as optional dependency (using required => 0) so that we
> don't bring in additional modules but still offer then as suggested in
> Debian package.

Will change dependency patch accordingly. Thanks.

QA Comments (before Sign-off):

1/ Why don't you remove the old script? (check-url.pl)
2/ Please replace
  +use 5.010;
  +use strict;
  +use warnings;
with use Modern::Perl;
3/ It seems in Koha, the scripts have an extension. Please add the .pl
4/ It would be great to pass an argument (or syspref :-/) to the script containing a list of field to check ? In UNIMARC, we can have 316$u, 317$u, 464$u, etc. with urls.
5/ For a new script it is very nice to submit the patch after a perltidy ;)

1, 3 and 5 are mandatory to pass QA

> 1/ Why don't you remove the old script? (check-url.pl)

To avoid any regression to any library using the old script in a way I can't
figure out. Maybe later?

> 3/ It seems in Koha, the scripts have an extension. Please add the .pl 

I don't like this rule but I can follow it...

> 4/ It would be great to pass an argument (or syspref :-/) to the script
> containing a list of field to check ? In UNIMARC, we can have 316$u, 317$u,
> 464$u, etc. with urls.

Good suggestion.

> 5/ For a new script it is very nice to submit the patch after a perltidy ;)

Will send a new version in few weeks.

Hi Frédéric,

Thank you for submitting this patch such fast :)
It works very well in all my tests.

Just somes little remarks:
- POD is not up to date (--tags in summary, -h is not supported, "Scan all URLs found in 856$u" or in others tags :))
- Perhaps a modification in the other script is necessary (to indicate that an other one is available).
- I think the following lines
    my %h = map { $_ => undef } @tags;
    @tags = sort keys %h;
could be replaced with:
    @tags = sort @tags;

(In reply to comment #15)
> Hi Frédéric,
> 
> Thank you for submitting this patch such fast :)
> It works very well in all my tests.
> 
> Just somes little remarks:

status modded to 'in discussion'

> - POD is not up to date (--tags in summary, -h is not supported, "Scan all
> URLs found in 856$u" or in others tags :))
> - Perhaps a modification in the other script is necessary (to indicate that
> an other one is available).

Thanks. I will submit an improved version of this patch.

> - I think the following lines
>     my %h = map { $_ => undef } @tags;
>     @tags = sort keys %h;
> could be replaced with:
>     @tags = sort @tags;

No. This 2 lines are there to deduplicate tags, in any case...

Created attachment 11840 [details] [review]
Bug 7963 Parallel HTTP requests when checking URLs

Current script check-url.pl checks URL found in 856$u by sending HTTP
requests, one by one. The next request can't be sent before the previous
one get a result, which can be very slow for dead URL. I propose a new
script which send multiple requests simultaneously which improve
drastically URL checking execution time.

This script is based on AnyEvent and AnyEvent::HTTP CPAN modules.
Add new dependencies AnyEvent & AnyEvent::HTTP.

See doc: perldoc check-url-quick.pl

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>

QA comment:
 * passes koha-qa.pl
 * badly fail if you don't have AnyEvent packages installed. I wrote a small follow-up for a better message in case of problem. Please sign it, and mark passed QA, everything is OK

Created attachment 12648 [details] [review]
[SIGNED-OFF] follow-up: die nicely if AnyEvent libraries not installed

Good point!

Patch pushed to master. Could usefully be ported to 3.8

I'm thinking about using AnyEvent::HTTP for bug 10662, and I see that libanyevent-http-perl is now a part of Debian Stable (Jessie) and Debian Old Stable (Wheezy).

https://packages.debian.org/wheezy/libanyevent-http-perl

I wonder if we should now include it as a required rather than optional dependency? 

For my purposes, I could probably just display a message telling users that they need to install the library before they can use the feature, but it'd probably be nice to have it already available.

That said, I haven't decided whether or not to use AnyEvent::HTTP quite yet. Just one of a few options I'm pondering... but thought I'd mention how it's now in Debian.

In hindsight, it looks like libanyevent-perl was available in Squeeze, so they were halfway there already.

https://packages.debian.org/squeeze/libanyevent-perl