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

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

Github user markap14 commented on the issue:

    https://github.com/apache/nifi/pull/2777
  
    In hindsight, a "Context" type of object would have been a good idea in 
this situation, had it been done that way in the first place. However, while 
it's a helpful design pattern, we have to be cognizant of the fact that we 
always have performed a release that contains the Service API the way that it 
is. We need to avoid changing that, as any custom Controller Services would 
break if we change the API in a non-backward-compatible way. It would also 
break any custom processors that try to use a Lookup Service.
    While we do have some flexibility in changing Service API's after they have 
been released, it is kind of a nuclear option that should be avoided if we can. 
Especially for something that is as widely used as the lookup stuff.
    This is largely why I want to avoid also including attributes in the same 
map - it would potentially break existing services if new things suddenly are 
fed into that map that are not part of the lookup's coordinates. By introducing 
the `Map<String, String> context` and providing a default implementation in the 
interface, all existing services will work as they previously did, and all 
existing processors will work as the previously did. However, new or updated 
implementations will still be able to take advantage of FlowFile Attributes.


> LookupRecord should supply flowfile attributes to the lookup service
> --------------------------------------------------------------------
>
>                 Key: NIFI-5287
>                 URL: https://issues.apache.org/jira/browse/NIFI-5287
>             Project: Apache NiFi
>          Issue Type: Improvement
>            Reporter: Mike Thomsen
>            Assignee: Mike Thomsen
>            Priority: Major
>
> -LookupRecord should supply the flowfile attributes to the lookup service. It 
> should be done as follows:-
>  # -Provide a regular expression to choose which attributes are used.-
>  # -The chosen attributes should be foundation of the coordinates map used 
> for the lookup.-
>  # -If a configured key collides with a flowfile attribute, it should 
> override the flowfile attribute in the coordinate map.-
> Mark had the right idea:
>  
> I would propose an alternative approach, which would be to add a new method 
> to the interface that has a default implementation:
> {{default Optional<T> lookup(Map<String, Object> coordinates, Map<String, 
> String> context) throws LookupFailureException \{ return lookup(coordinates); 
> } }}
> Where {{context}} is used for the FlowFile attributes (I'm referring to it as 
> {{context}} instead of {{attributes}} because there may well be a case where 
> we want to provide some other value that is not specifically a FlowFile 
> attribute). Here is why I am suggesting this:
>  * It provides a clean interface that properly separates the data's 
> coordinates from FlowFile attributes.
>  * It prevents any collisions between FlowFile attribute names and 
> coordinates.
>  * It maintains backward compatibility, and we know that it won't change the 
> behavior of existing services or processors/components using those services - 
> even those that may have been implemented by others outside of the Apache 
> realm.
>  * If attributes are passed in by a Processor, those attributes will be 
> ignored anyway unless the Controller Service is specifically updated to make 
> use of those attributes, such as via Expression Language. In such a case, the 
> Controller Service can simply be updated at that time to make use of the new 
> method instead of the existing method.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to