nifi-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "ASF GitHub Bot (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (NIFI-1686) NiFi is unable to populate over 1/4 of AMQP properties from flow properties
Date Sun, 27 Mar 2016 15:56:26 GMT

    [ https://issues.apache.org/jira/browse/NIFI-1686?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15213504#comment-15213504
] 

ASF GitHub Bot commented on NIFI-1686:
--------------------------------------

Github user olegz commented on the pull request:

    https://github.com/apache/nifi/pull/305#issuecomment-202095259
  
    @steveyh25 
    
    Thank you so much for your contribution. AMQP support is one of the most recent additions
to NiFi and it's great to see its adoption is on the way. What's even more admirable is to
see the 
    members (such as yourself) of the community are eager to help making it even better by
addressing some of the limitations these new components may have.
    
    That said, keep in mind that certain limitations (especially in new components) may be
there intentionally (as in this case). In other words this initial version of AMQP was not
supporting _non-String-value-type_ properties since we wanted to give more thoughts as to
the conversion strategies of such values to/from String given that NiFi attributes only support
String as a value type. I'll comment more on this inline at the relevant parts of the code,
but wanted to make sure that it is clear that such behavior was intentional and not a bug.
I'll update JIRA classification accordingly as well. 
    
    Also, instead of saying "1/4" consider listing _timestamp, deliveryMode, headers, priority_
individually. The main point here is that one only cares about properties he/she needs and
if the other 3/4 satisfy those needs then all is good while it may not be all good for others
who need one of the unsupported properties. So the emphasis here should be on specific property
rather then 1/4. 
    
    Last but not least, consider the tone and basic ethics next time you interact in a public
forum (JIRA/Github etc.). When writing things like "the author of the code mustn't have. .
." keep in mind that 
    you can not possibly know _what_, _where_ and _how_ in relation to the "author's" _intent_,
_thoughts_ and/or _motivation_ unless you had direct interaction with him/her which I'll assume
you didn't. Without such interaction these words become pure speculation which personalizes
the issue and therefore counter productive. See more here: http://www.apache.org/foundation/policies/conduct.html
    
    That said, please see the comments in line as I go through the review.
    
    Once again thank you for you contribution!


> NiFi is unable to populate over 1/4 of AMQP properties from flow properties
> ---------------------------------------------------------------------------
>
>                 Key: NIFI-1686
>                 URL: https://issues.apache.org/jira/browse/NIFI-1686
>             Project: Apache NiFi
>          Issue Type: Bug
>          Components: Core Framework
>    Affects Versions: 0.5.1
>            Reporter: Stephen Harper
>
> When creating a flow (we used ListenHTTP, but this bug will affect all) that forwards
on to a rabbit queue, org.apache.nifi.amqp.processors.PublishAMQP uses the method extractAmqpPropertiesFromFlowFile
to populate the AMQP BasicProperties if the flow attributes match a certain format (i.e amqp$contentType=text/xml).

> The method in question uses reflection to find a matching method name from the AMQP.BasicProperties
class, and tries to populate accordingly.
> This works fine for all properties that take a String argument - however there are some
that don't (specifically, headers takes a Map<String, Object>, deliveryMode and priority
take Integer, and timestamp takes a Date), and it is impossible to populate these values because
the invocation assumes a String is required, and fails on line 210.
> Whatsmore, the comment underneath (line 215) states that "this should really never happen
since it should be caught by the above IF" - however the author of the code mustn't have tested
all cases because this error is consistently present when trying to forward flow attributes
in over a quarter of the available amqp properties.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message