Bug 878940

Summary: spout/drain examples fixes and improvements
Product: Red Hat Enterprise MRG Reporter: Petr Matousek <pematous>
Component: perl-qpidAssignee: messaging-bugs <messaging-bugs>
Status: VERIFIED --- QA Contact: Petr Matousek <pematous>
Severity: medium Docs Contact:
Priority: medium    
Version: DevelopmentCC: jross
Target Milestone: ---Keywords: Patch
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: perl-qpid-0.18-2 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
patch for spout.pl
none
patch for drain.pl
none
Adds fixes to spout.pl and drain.pl none

Description Petr Matousek 2012-11-21 15:30:07 UTC
Description of problem:

It is unable to set message properties with spout.pl, following change needs to be made to fix this issue:
  - $message->getProperties()->{$name} = $value;
  + $message->setProperty($name, $value);

Below are proposed changes to the spout/drain examples to be more conformable to other clients (c++, python, java), I think it is valuable to have all the client examples working the same way:

Patch files for both spout.pl/drain.pl can be found in attachment.

* drain.pl *:

Drain do not output the message properties in usable form (properties are displayed as a hash), ie:
# perl /usr/share/doc/perl-qpid-0.18/examples/drain.pl -c 1 q
Message(properties=HASH(0x8f1ecc8),content='')

It also can be useful to display if the message is redelivered as other clients do.

Proposing following change to fix these issues:
  +sub printProperties {
  +  my $h = shift();
  +  return qq[{${\(join', ',map"'$_': '$h->{$_}'",keys%$h)}}]
  +}

  - print "Message(properties=" . $message->getProperties() . ",content='";
  + my $redelivered = ($message->getRedelivered) ? "redelivered=True, " : "";
  + print "Message(" . $redelivered . "properties=" . printProperties($message->getProperties()) . ", content='";

Proposing to set default timeout to 0 (to be more conformable to other clients):
  - my $timeout = 60;
  + my $timeout = 0;

Proposing to set default messages count to 0 (to be more conformable to other clients)
  - my $count   = 1;
  + my $count   = 0;

* both spout.pl/drain.pl *:

It is good practise to output usage with -h/--help options, or if the input is wrong. It is also in place to stop the program execution after the bad input/help request.

Proposing following change:
  -    "connection-options=s" => \ $connectionOptions,   
  -);
  -
  -
  -if (! $result) {
  -    print "Usage: perl drain.pl [OPTIONS]\n";
  -}
  -
  +    "connection-options=s" => \ $connectionOptions,
  +    "help|h"               => \ &usage
  +) or usage();

  +sub usage {
  +    print "Usage: perl drain.pl [OPTIONS]\n";
  +    exit;
  +}

Version-Release number of selected component (if applicable):
perl-qpid-0.18-1.el6

Actual results:
Unable to set message properties with spout.pl, spout/drain behaviour differs from other clients (c++,py,java)

Expected results:
Message properties can be set using spout.pl, spout/drain behaviour more concords to other clients

Comment 1 Petr Matousek 2012-11-21 15:32:00 UTC
Created attachment 649277 [details]
patch for spout.pl

Comment 2 Petr Matousek 2012-11-21 15:32:49 UTC
Created attachment 649278 [details]
patch for drain.pl

Comment 3 Darryl L. Pierce 2012-11-26 18:11:40 UTC
I think, on top of this, the usage subroutine ought to include help on the actual options available. Simply showing "[OPTIONS]" isn't very helpful.

Comment 4 Petr Matousek 2012-11-26 18:43:50 UTC
(In reply to comment #3)
> I think, on top of this, the usage subroutine ought to include help on the
> actual options available. Simply showing "[OPTIONS]" isn't very helpful.

Agreed +1

Comment 5 Darryl L. Pierce 2012-11-26 19:31:45 UTC
I've written a few additions that use Pod::Usage to show help information. If you like this, I'll go ahead and commit the patch.

Comment 6 Darryl L. Pierce 2012-11-26 19:32:54 UTC
Created attachment 652226 [details]
Adds fixes to spout.pl and drain.pl

Comment 7 Petr Matousek 2012-11-27 14:52:27 UTC
(In reply to comment #5)
> I've written a few additions that use Pod::Usage to show help information.
> If you like this, I'll go ahead and commit the patch.

Hi Darryl, the changes looks good to me, please go ahead and commit the patch. Thanks.

Just a small detail - you provide the following options "help|h|?" to display the help using spout, drain uses only "help|h" ('?' omitted).

Comment 8 Darryl L. Pierce 2012-11-27 15:09:19 UTC
(In reply to comment #7)
> (In reply to comment #5)
> > I've written a few additions that use Pod::Usage to show help information.
> > If you like this, I'll go ahead and commit the patch.
> 
> Hi Darryl, the changes looks good to me, please go ahead and commit the
> patch. Thanks.
> 
> Just a small detail - you provide the following options "help|h|?" to
> display the help using spout, drain uses only "help|h" ('?' omitted).

Okay, fixed that before pushing by removing ? from spout to keep it consistent.

Comment 11 Petr Matousek 2013-01-04 13:45:31 UTC
This issue has been resolved.
 
Verified on rhel6.3 (x86_64, i386) - perl-qpid-0.18-2.

-> VERIFIED

Comment 13 Petr Matousek 2013-05-15 13:35:55 UTC
(In reply to comment #11)
> This issue has been resolved.
>  
> Verified on rhel6.3 (x86_64, i386) - perl-qpid-0.18-2.
> 
> -> VERIFIED

-> Moving back to ON_QA in order to verify on 0.22 packages.

Comment 15 Petr Matousek 2014-05-05 12:00:12 UTC
Although it was not possible to verify exactly the requested changes (because a new qpid::messaging perl layer was added above the c++ swig bindings). All the reported issues are fixed in the new implementation.

This issue has been fixed. Verified on rhel6.5 (x86_64, i386).

packages under test:
perl-qpid-0.22-11

-> VERIFIED