[Bug 7963] New: Parallel HTTP requests when checking URLs
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Priority: P5 - low Change sponsored?: --- Bug ID: 7963 Assignee: gmcharlt@gmail.com Summary: Parallel HTTP requests when checking URLs QA Contact: koha.sekjal@gmail.com Severity: enhancement Classification: Unclassified OS: All Reporter: frederic@tamil.fr Hardware: All Status: NEW Version: rel_3_8 Component: Command-line Utilities Product: Koha Current script check-url.pl checks URL found in 856$u by sending HTTP requests, one by one. I propose a new script which send multiple requests simultaneously. This script is based on AnyEvent and AnyEvent::HTTP CPAN modules. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |2959, 6267 Assignee|gmcharlt@gmail.com |frederic@tamil.fr -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 --- Comment #1 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 9195 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=9195&action=edit Proposed patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #9195|0 |1 is obsolete| | --- Comment #2 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 9197 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=9197&action=edit Proposed patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #9197|0 |1 is obsolete| | --- Comment #3 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 9206 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=9206&action=edit Proposed patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Dobrica Pavlinusic <dpavlin@rot13.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dpavlin@rot13.org --- Comment #4 from Dobrica Pavlinusic <dpavlin@rot13.org> --- 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 are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #9206|0 |1 is obsolete| | --- Comment #5 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 10194 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10194&action=edit Proposed patch Thanks Dobrica for your comments that make sense. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 --- Comment #6 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 10195 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10195&action=edit Dependencies -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 --- Comment #7 from Dobrica Pavlinusic <dpavlin@rot13.org> --- 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 are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 --- Comment #8 from Frédéric Demians <frederic@tamil.fr> ---
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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10194|0 |1 is obsolete| | --- Comment #9 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 10238 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10238&action=edit Proposed patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10195|0 |1 is obsolete| | --- Comment #10 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 10239 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=10239&action=edit Dependencies -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@biblibre.co | |m --- Comment #11 from Jonathan Druart <jonathan.druart@biblibre.com> --- 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 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 --- Comment #12 from Frédéric Demians <frederic@tamil.fr> ---
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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #10238|0 |1 is obsolete| | Attachment #10239|0 |1 is obsolete| | --- Comment #13 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 11403 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=11403&action=edit Proposed patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #11403|0 |1 is obsolete| | --- Comment #14 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 11405 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=11405&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 --- Comment #15 from Jonathan Druart <jonathan.druart@biblibre.com> --- 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; -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Mason James <mtj@kohaaloha.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion CC| |mtj@kohaaloha.com --- Comment #16 from Mason James <mtj@kohaaloha.com> --- (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' -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 --- Comment #17 from Frédéric Demians <frederic@tamil.fr> ---
- 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... -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #11405|0 |1 is obsolete| | --- Comment #18 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 11837 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=11837&action=edit Proposed patch -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #11837|0 |1 is obsolete| | --- Comment #19 from Jonathan Druart <jonathan.druart@biblibre.com> --- Created attachment 11840 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=11840&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Jonathan Druart <jonathan.druart@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |paul.poulain@biblibre.com --- Comment #20 from Paul Poulain <paul.poulain@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 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 --- Comment #21 from Paul Poulain <paul.poulain@biblibre.com> --- Created attachment 12644 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=12644&action=edit Bug 7963 follow-up: die nicely if AnyEvent libraries not installed -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #12644|0 |1 is obsolete| | --- Comment #22 from Frédéric Demians <frederic@tamil.fr> --- Created attachment 12648 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=12648&action=edit [SIGNED-OFF] follow-up: die nicely if AnyEvent libraries not installed Good point! -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to Master --- Comment #23 from Paul Poulain <paul.poulain@biblibre.com> --- Patch pushed to master. Could usefully be ported to 3.8 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Chris Cormack <chris@bigballofwax.co.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |chris@bigballofwax.co.nz Version|rel_3_8 |rel_3_10 -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 Bug 7963 depends on bug 2959, which changed state. Bug 2959 Summary: Enhancements for link checker http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=2959 What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #24 from David Cook <dcook@prosentient.com.au> --- 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. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=7963 --- Comment #25 from David Cook <dcook@prosentient.com.au> --- In hindsight, it looks like libanyevent-perl was available in Squeeze, so they were halfway there already. https://packages.debian.org/squeeze/libanyevent-perl -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org