[GitHub] nifi issue #2527: FetchHBaseRow - log level and displayName

2018-03-12 Thread bbende
Github user bbende commented on the issue:

https://github.com/apache/nifi/pull/2527
  
I think when I initially implemented this processor I was assuming someone 
would be fetching rows that were expected to be there, so if it wasn't there 
then it would be more an error scenario.

However, looking back on it now... since we do have the explicit not found 
relationship, it is going to be obvious when something is not found, and we can 
use LogMessage as Pierre mentioned if we need to generate a bulletin there. So 
I'm ok with changing it to debug.


---


[GitHub] nifi issue #2527: FetchHBaseRow - log level and displayName

2018-03-11 Thread bdesert
Github user bdesert commented on the issue:

https://github.com/apache/nifi/pull/2527
  
@pvillard31 , @bbende In case we want to have bulletin to be generated, we 
can change "Bulletin Level" of the component to "DEBUG". But then bulletin will 
be generated even if rowkey is found. I would rather remove lines 276-278, to 
have clean debug level bulletin for "not found" only.
Your thoughts? 


---


[GitHub] nifi issue #2527: FetchHBaseRow - log level and displayName

2018-03-10 Thread pvillard31
Github user pvillard31 commented on the issue:

https://github.com/apache/nifi/pull/2527
  
Thought a bit more about this one and I wondered if someone could be in a 
situation where they really want a bulletin to be generated when the row is not 
found. But since the flow file will be routed to a specific relationship 
(REL_NOT_FOUND), they can actually generate a bulletin using another processor 
(like LogMessage) for this relationship. Another option would be to let the 
user choose the behavior with a property but it sounds a bit overcomplicated... 
What do you think @bbende? (you created this processor, I'm curious to have 
your opinion)


---


[GitHub] nifi issue #2527: FetchHBaseRow - log level and displayName

2018-03-09 Thread bdesert
Github user bdesert commented on the issue:

https://github.com/apache/nifi/pull/2527
  
oh, I see now. so it was about my first commit, when I added "displayName" 
and changed the "name" as per standard. But after Pierre's comment removed 
those fixes.
Thanks for taking a look anyway!


---


[GitHub] nifi issue #2527: FetchHBaseRow - log level and displayName

2018-03-09 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2527
  
@bdesert When I read the commit, I wasn't sure what regression it was 
actually fixing.


---


[GitHub] nifi issue #2527: FetchHBaseRow - log level and displayName

2018-03-09 Thread bdesert
Github user bdesert commented on the issue:

https://github.com/apache/nifi/pull/2527
  
@MikeThomsen , I think since it is already in 1.5 and ppl started using it, 
it could be a case, when implementation can use rest APIs to work with these 
processors. And since we are changing "name" property (which cannot be 
controlled by user), it can create a problem with those eixsting scripts. Also, 
name is a part of flow.xml.gz, that will also impact existing implementations 
and make regression impact. So I had to agree to Pierre comment about 
regression.


---


[GitHub] nifi issue #2527: FetchHBaseRow - log level and displayName

2018-03-09 Thread MikeThomsen
Github user MikeThomsen commented on the issue:

https://github.com/apache/nifi/pull/2527
  
What sort of regression was this supposed to address?


---


[GitHub] nifi issue #2527: FetchHBaseRow - log level and displayName

2018-03-09 Thread pvillard31
Github user pvillard31 commented on the issue:

https://github.com/apache/nifi/pull/2527
  
Hi @bdesert - unfortunately, since this processor has been already released 
with NiFi 1.5.0, we cannot change the name of the properties in a minor release 
as it would break the existing workflows. That's why we encourage the use of 
both name() and displayName() when a new processor is created so that we can 
change the displayName() value without impacting existing workflows. I think 
you can just drop the changes for properties name and just leave the log level 
change.


---