NIFI-5214 Added REST LookupService #2723

MikeThomsen · 2018-05-19T13:25:42Z

Thank you for submitting a contribution to Apache NiFi.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
Has your PR been rebased against the latest commit within the target branch (typically master)?
Is your initial contribution a single, squashed commit?

For code changes:

Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
Have you written or updated unit tests to verify your changes?
If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?

For documentation related changes:

Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.

ottobackwards · 2018-05-19T14:15:22Z

This is cool, I will definitely make an aws web api version of this after it and my pr lands.
I think that any rest service needs to support the options that InvokeHttp supports. Proxies etc. This doesn't seem to do that.

MikeThomsen · 2018-05-19T14:25:51Z

InvokeHttp has a lot of options, so that might be worth splitting it into an initial merge and then an improvement that comes in 1.8.

ottobackwards · 2018-05-19T14:31:35Z

Don't I know it. If you want to do this as a 'series' of prs, I think you should document that in jira with tasks representing each PR and note it clearly in the pr.

That will set the right context for the reviewers.

I chose with my InvokeAwsGatewayApi processor to make sure that I replicated the InvokeHttp test suite. I would suggest that the end goal here would be to have and extensive set of tests adapted from that suite for the reader.

MikeThomsen · 2018-05-20T12:15:06Z

@ottobackwards I grabbed a bunch of code from InvokeHttp and did a little copy pasta. Please take a look when you get a chance. I think moving the copy pasta into a shared utility package will need to be a separate ticket in order to keep the ticket scope down.

MikeThomsen · 2018-05-20T18:17:12Z

@ottobackwards I think I might just go ahead and refactor that code now and get it done in this PR.

MikeThomsen · 2018-05-20T22:50:37Z

@ottobackwards @ijokarumawak Added support for the new ProxyConfigurationService. Would appreciate a review.

ottobackwards

First couple of things

ottobackwards · 2018-05-21T10:42:06Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
+


Should this be tagged HTTP?

ottobackwards · 2018-05-21T10:44:46Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+        if (proxyConfigurationService != null) {
+            setProxy(builder);
+        }
+


why are we getting the SSL_CONTEXT_SERVICE twice?

ottobackwards · 2018-05-21T10:56:58Z

@MikeThomsen
Here are the properties exposed for configuration of InvokeHttp:

 public static final List<PropertyDescriptor> PROPERTIES = Collections.unmodifiableList(Arrays.asList(
            PROP_METHOD,
            PROP_URL,
            PROP_SSL_CONTEXT_SERVICE,
            PROP_CONNECT_TIMEOUT,
            PROP_READ_TIMEOUT,
            PROP_DATE_HEADER,
            PROP_FOLLOW_REDIRECTS,
            PROP_ATTRIBUTES_TO_SEND,
            PROP_BASIC_AUTH_USERNAME,
            PROP_BASIC_AUTH_PASSWORD,
            PROP_PROXY_HOST,
            PROP_PROXY_PORT,
            PROP_PROXY_TYPE,
            PROP_PROXY_USER,
            PROP_PROXY_PASSWORD,
            PROP_PUT_OUTPUT_IN_ATTRIBUTE,
            PROP_PUT_ATTRIBUTE_MAX_LENGTH,
            PROP_DIGEST_AUTH,
            PROP_OUTPUT_RESPONSE_REGARDLESS,
            PROP_TRUSTED_HOSTNAME,
            PROP_ADD_HEADERS_TO_REQUEST,
            PROP_CONTENT_TYPE,
            PROP_SEND_BODY,
            PROP_USE_CHUNKED_ENCODING,
            PROP_PENALIZE_NO_RETRY));

Of these, I wonder if we should consider for the rest lookup
PROP_CONNECT_TIMEOUT,
PROP_READ_TIMEOUT,
PROP_DATE_HEADER,
PROP_FOLLOW_REDIRECTS,
PROP_ATTRIBUTES_TO_SEND,
PROP_TRUSTED_HOSTNAME,
PROP_ADD_HEADERS_TO_REQUEST
In whatever form makes sense for the lookup service.

For example: what if my lookup service is someone else's API and I need to send custom headers and api keys?

ottobackwards · 2018-05-21T11:07:14Z

I almost wonder if there should be an http rest connection service

MikeThomsen · 2018-05-21T12:58:59Z

@ottobackwards I'll add user-defined headers and basic auth support, but that's it until there's a discussion around a HTTP Connection Service. I think that would be the place to go crazy on the options, not the lookup service.

ottobackwards · 2018-05-21T13:02:56Z

@MikeThomsen I agree. I haven't seen any conversation on it, but it seems more and more obvious, given the way things are going right?

ijokarumawak

@MikeThomsen This new capability would be really useful. Thanks! I looked through the new lines of code and posted few comments. Please check those.

ijokarumawak · 2018-05-22T00:42:03Z

...ices/src/main/resources/docs.org.apache.nifi.lookup.RestLookupService/additionalDetails.html

@@ -0,0 +1,42 @@
+<!DOCTYPE html>


@MikeThomsen additionalDetails.html should be in a directory src/main/resources/docs/qualifiedClassName/ not in docs.qualifiedClassName. Please check additional docs can be accessed from component's usage page from NiFi UI.

ijokarumawak · 2018-05-22T00:54:17Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+                e -> e.getKey(),
+                e -> e.getValue().toString()
+            ));
+        RecordReader reader = readerFactory.createRecordReader(variables, is, getLogger());


Please use try-with-resource statement, similar to what AbstractRouteRecord does:

try (final RecordReader reader = readerFactory.createRecordReader(originalAttributes, in, getLogger())) {

ijokarumawak · 2018-05-22T00:55:08Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+                e -> e.getValue().toString()
+            ));
+        RecordReader reader = readerFactory.createRecordReader(variables, is, getLogger());
+        Record record = reader.nextRecord();


We should handle the case where record is null.

Ok, not sure how to trick it into doing null, but I put a check for that and made the optional be empty and added a new unit test to handle empty records (ex. badly defined schema has no fields from a rest response).

ijokarumawak · 2018-05-22T01:20:23Z

...undle/nifi-lookup-services/src/test/groovy/org/apache/nifi/lookup/RestLookupServiceIT.groovy

+
+            def coordinates = [
+                "schema.name": "simple",
+                "endpoint": server.url + "/simple",


In real use-cases, do we expect user to set 'server.url' at a LookupRecord processor's user defined property?

As an alternative approach, I'd add endpoint url at RestLookupService to define a template string to resolve target endpoint, and let callers such as LookupRecord to pass required variables to complete the target endpoint.

For example:

At RestLookupService

Endpoint URL: https://some-api.example.com/buckets/${bucketId}/items/${itemId}

At LookupRecord processor

bucketId as user-defined-property: ./bucketId (a record path to get a value)

itemId as user-defined-property: ./itemId (a record path to get a value)

By doing so, it also supports passing the entire endpoint URL:

At RestLookupService

Endpoint URL: ${endpoint}

At LookupRecord processor

endpoint as user-defined-property: a record path to construct an URL

I'd expect RestLookupService to have more control on REST related configurations, and callers just pass variables to make actual requests. How do you think?

I like that. How about this breakdown:

endpoint is a template string set on a property descriptor and we use an EL compiler to generate the endpoint on the fly with those lookup coordinates.

Add direct_endpoint which is a treated as literal value to override that if present.

If both are present, throw an exception.

Thoughts?

@MikeThomsen I've replied to your question on the dev ML. I think endpoint property can be used both cases where user want to use EL or just a literal endpoint. Because Query.prepare method can return the configured literal String if it does not contain any EL.

final Map<String, String> map = Collections.singletonMap("name", "John Smith"); PreparedQuery query = Query.prepare("${name}-${name:length()}"); String result = query.evaluateExpressions(map, null); System.out.println(result); query = Query.prepare("name-name:length()"); result = query.evaluateExpressions(map, null); System.out.println(result); final Map<String, String> map = Collections.singletonMap("name", "John Smith"); PreparedQuery query = Query.prepare("${name}-${name:length()}"); String result = query.evaluateExpressions(map, null); System.out.println(result); query = Query.prepare("name-name:length()"); result = query.evaluateExpressions(map, null); System.out.println(result);

The code prints:

John Smith-10 name-name:length()

MikeThomsen · 2018-06-01T12:58:27Z

@ijokarumawak I can't believe I missed your comments. Will try to get to those today.

MikeThomsen · 2018-06-01T19:06:16Z

@ijokarumawak Your first feedback points were merged in, let me know what you think of the last item and it should be easy to get done.

markap14

@MikeThomsen this is a good idea - i think it will be extremely helpful for a lot of use cases. I left several inline comments. A couple I think are actually bugs (a 'fall through' case if RecordPath doesn't match and a threading issue). The others are mostly just regarding general NiFi coding conventions. Thanks!

markap14 · 2018-06-01T19:21:09Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+        .description("An optional record path that can be used to define where in a record to get the real data to merge " +
+                "into the record set to be enriched. See documentation for examples of when this might be useful.")
+        .expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
+        .addValidator(Validator.VALID)


Should use a RecordPath validator, no?

Ok. Didn't know it even existed :)

markap14 · 2018-06-01T19:21:31Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+        .description("The record reader to use for loading the payload and handling it as a record set.")
+        .expressionLanguageSupported(ExpressionLanguageScope.NONE)
+        .identifiesControllerService(RecordReaderFactory.class)
+        .addValidator(Validator.VALID)


No need for the explicit validator when identifying controller services.

markap14 · 2018-06-01T19:24:00Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+        getHeaders(context);
+    }
+
+    private Map<String, String> headers;


Typical convention in NiFi is to add member variables at the top of the class before constructors.

markap14 · 2018-06-01T19:26:22Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+    private void getHeaders(ConfigurationContext context) {
+        headers = new HashMap<>();
+        for (PropertyDescriptor descriptor : context.getProperties().keySet()) {
+            if (descriptor.getName().startsWith("header.")) {


If dynamic properties are used for specifying headers to use, why not just let the property name be the name of the header? I.e., why require a "header." prefix? We should also document how dynamic properties are used via the @dynamicproperties annotation.

markap14 · 2018-06-01T19:27:44Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+
+            Record record = handleResponse(is, coordinates);
+
+            return record != null


Can just use Optional.ofNullable(record) here

markap14 · 2018-06-01T19:33:39Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+                    } else if (value instanceof Map) {
+                        temp = new MapRecord(schema, (Map<String, Object>) value);
+                    } else {
+                        temp = new MapRecord(schema, new HashMap<String, Object>() {{


We should avoid the creation of an anonymous inner class here and instead just create the MapRecord and then call put()

markap14 · 2018-06-01T19:35:31Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+                    FieldValue fieldValue = fv.get();
+                    RecordSchema schema = new SimpleRecordSchema(Arrays.asList(fieldValue.getField()));
+                    String[] parts = recordPath.getPath().split("/");
+                    String last = parts[parts.length - 1];


I don't think this is actually what we want here. What if the path is something like /myField[./second/third = 'hello'] - in that case, last will equal third = 'hello']

Great point. I think I'm going to have to make another property to handle this instead.

markap14 · 2018-06-01T19:37:04Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+            return record;
+        } catch (Exception ex) {
+            is.close();
+            throw new RuntimeException(ex);


Would avoid wrapping checked exceptions that are known to be thrown by method (such as IOException) in a RuntimeException. Am guessing you can just use throw ex because all checked exceptions that can be caught are likely already declared for the method but not sure off the top of my head.

markap14 · 2018-06-01T19:39:06Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+        return request.build();
+    }
+
+    private String basicUser;


We should avoid member variables throughout code and try to concentrate them at the top of the class. Additionally, these 3 member variables are not protected so the class is not thread-safe. Need to ensure that all ControllerServices are thread-safe.

I think I fixed it.

markap14 · 2018-06-01T19:42:34Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+
+    @Override
+    public Set<String> getRequiredKeys() {
+        return new HashSet<String>(){{


We should avoid the creation of anonymous inner classes when we can. We can also make this an UnmodifiableSet and just create a single instance of it as a member variable instead of constantly creating the Set

MikeThomsen · 2018-06-02T14:02:50Z

@markap14 I think your concerns are addressed now.

@ijokarumawak I tried out the expression language APIs and found that I can't just submit a Map, so I'm going to hold off on providing a dynamic template URL for now. That could be be a good improvement to add after the expression language compiler api gets updated to support generic maps as well (if it ever happens).

ijokarumawak

@MikeThomsen Commented on the use of EL with a Map above, please check that out. Also, found the use of company.com, let's change it to example.com.

Do you think making the URL as template can be included in this PR? I will wait in case if you want to do that with this PR. Please let me know, once I got your confirmation, I will do the final review & testing before merging.

Thanks!

ijokarumawak · 2018-06-04T01:38:13Z

...nifi-lookup-services/src/test/groovy/org/apache/nifi/lookup/rest/handlers/ComplexJson.groovy

+                        inner: [
+                            "username": "jane.doe",
+                            "password": "testing7890",
+                            "email": "jane.doe@company.com"


The domain company.com actually exists. Please change it to example.com.

ijokarumawak · 2018-06-04T01:38:24Z

...undle/nifi-lookup-services/src/test/groovy/org/apache/nifi/lookup/RestLookupServiceIT.groovy

+            def record = response.get()
+            Assert.assertEquals("jane.doe", record.getAsString("username"))
+            Assert.assertEquals("testing7890", record.getAsString("password"))
+            Assert.assertEquals("jane.doe@company.com", record.getAsString("email"))


The domain company.com actually exists. Please change it to example.com.

MikeThomsen · 2018-06-04T11:17:54Z

@ijokarumawak I'll take a stab at adding the template url option.

ottobackwards · 2018-06-04T12:25:06Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

@@ -277,6 +280,20 @@ private void setProxy(OkHttpClient.Builder builder) {
        }
    }



Should this handle AttributeExpressionLanguageParsingException? Is there any validation that can be done?

I don't think so because the EL support is enabled on the fly by the user, and any user who is enterprising enough to do that should be responsible for validating it. There's no good fallback option short of having them specify yet another key, and IMO that's overkill.

ijokarumawak · 2018-06-05T00:31:28Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+                    e -> e.getKey(),
+                    e -> e.getValue().toString()
+                ));
+            final PreparedQuery query = Query.prepare((String)coordinates.get(ENDPOINT_KEY));


@MikeThomsen Thanks for adding this! This gets closer to what I was imagined. Excuse me if I was not clear enough, but from a user's stand point, I'd like to configure this ENDPOINT_KEY value at a RestLookupService property rather than passing it via coordinates.

The template it self can be configured at the service statically, but the variables to resolve a complete endpoint URL can be passed as coordinates.

Because, I think people would use one RestLookupService to lookup some endpoints (can be more than 1) of a particular Web service API. If they need to deal with different Web services, e.g. Twitter, Facebook and Salesforce ...etc they would use 3 RestLookupService, and each one can have the endpoint template defined.

Further more, if we implemented like that, we can cache the PreparedQuery instance without parsing the EL every time. Also, no need to have ENDPOINT_TEMPLATE_KEY coordinate to tell if it contains EL or not because the PreparedQuery returns the original String if it does not contains EL.

How do you think?

I think that would work.

ijokarumawak

@MikeThomsen I was able to use RestLookupService from LookupRecord processor. However, found several places for further improvements. Please check the comments. Thanks!

ijokarumawak · 2018-06-06T12:14:09Z

nifi-nar-bundles/nifi-standard-bundle/nifi-standard-web-utils/pom.xml

+        <version>1.7.0-SNAPSHOT</version>
+    </parent>
+    <modelVersion>4.0.0</modelVersion>
+    <artifactId>nifi-standard-web-utils</artifactId>


I think extracting TestServer.java was not necessary, and I'd avoid doing this in this PR. Because:

It makes this PR to touch many files unnecessarily in terms of its objective.

Having test scope dependency to nifi-standard-processors should be fine to reuse TestServer.

Even if we do extract TestServer, the module name nifi-standard-web-utils will not be appropriate to contain Test utility class. It should be named more clearly to state that this module is for test. nifi-mock is a good example.

Do you have a strong motivation to do this?

Yeah. I can see this functionality being reused in plenty of places and don't think we should tie the transitive dependencies of the standard package's test scope into other packages as that could complicate things down the road. I'll change the module name to nifi-standard-web-test-utils. How about that?

ijokarumawak · 2018-06-06T12:24:50Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+        .description("The username to be used by the client to authenticate against the Remote URL.  Cannot include control characters (0-31), ':', or DEL (127).")
+        .required(false)
+        .addValidator(StandardValidators.createRegexMatchingValidator(Pattern.compile("^[\\x20-\\x39\\x3b-\\x7e\\x80-\\xff]+$")))
+        .build();


This should support EL with Variable Registry.

ijokarumawak · 2018-06-06T12:24:59Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+        .required(false)
+        .sensitive(true)
+        .addValidator(StandardValidators.createRegexMatchingValidator(Pattern.compile("^[\\x20-\\x7e\\x80-\\xff]+$")))
+        .build();


This should support EL with Variable Registry.

ijokarumawak · 2018-06-06T12:36:55Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+        // If the username/password properties are set then check if digest auth is being used
+        if (!authUser.isEmpty() && isDigest) {
+            final String authPass = trimToEmpty(context.getProperty(PROP_BASIC_AUTH_PASSWORD).getValue());
+            this.basicPass = authPass;


I think this.basicPass should be set regardless of isDigest, similar to this.basicUser. Having it here will leave basicPass unset if isDigest is false. Is this correct?

ijokarumawak · 2018-06-06T12:55:42Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+    public Optional<Record> lookup(Map<String, Object> coordinates) throws LookupFailureException {
+        final String endpoint = determineEndpoint(coordinates);
+        final String mimeType = (String)coordinates.get(MIME_TYPE_KEY);
+        final String method   = (String)coordinates.get(METHOD_KEY);


Using getOrDefault(METHOD_KEY, "get") would be more user friendly.

A value is required because it's a required key, so I don't think that's necessary.

When I used the lookup service from LookupRecord, I felt passing a literal value as RecordPath was a bit tricky. I had to use concat('get').

@MikeThomsen RequiredKeys is not mandatory, it can be an empty Set. Also, this is a Lookup service. Get should be the default method like the famous cURL command.

I can see how that would be awkward. Do you want to handle that with a combination of property + lookup key (latter overriding the former) or defer to fixing LookupRecord?

I want RestLookupService to have 'get' as default value. It will cover 80% of use-cases I assume. I will send a PR to your PR and if it looks fine, then I'd give my +1 so that you can merge.

ijokarumawak · 2018-06-06T13:33:03Z

...ices/src/main/resources/docs/org.apache.nifi.lookup.RestLookupService/additionalDetails.html

+    <p>Headers are supported using dynamic properties. Just add a dynamic property and the name will be the header name and the value will be the value for the header. Expression language
+    powered by input from the variable registry is supported.</p>
+    <h2>Dynamic URLs</h2>
+    <p>The URL property supports expression language in a non-standard way: through the lookup key/value pairs configured on the processor. The configuration specified by the user will be passed


I'd omit the expression, 'in a non-standard way', because I envision this pattern will be used more frequently and I'm planning to add copy FlowFile attribute to coordinates capability at LookupRecord processor

ijokarumawak · 2018-06-06T13:34:22Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+    @DynamicProperty(name = "*", value = "*", description = "All dynamic properties are added as HTTP headers with the name " +
+            "as the header name and the value as the header value.")
+})
+public class RestLookupService extends AbstractControllerService implements LookupService<Record> {


implements RecordLookupService is better.

ijokarumawak · 2018-06-06T13:38:40Z

...ices-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/RestLookupService.java

+        Set<String> _keys = new HashSet<>();
+        _keys.add(MIME_TYPE_KEY);
+        _keys.add(METHOD_KEY);
+        KEYS = Collections.unmodifiableSet(_keys);


MIME_TYPE is only used for put and post requests and should be optional. Also if we support 'get' is the default method, then this controller service does not have any required KEYS.

That's a fair point, and it needs validation on the verbs.

MikeThomsen · 2018-06-09T12:06:18Z

Other than the issue w/ literals you cited, looks ready to merge IMO.

MikeThomsen · 2018-06-12T11:01:01Z

@ijokarumawak merged your PR. You can merge at any time.

MikeThomsen · 2018-06-12T11:03:45Z

@ijokarumawak the AWS gateway tests are broken because I moved test server. I'll do a patch for that.

MikeThomsen · 2018-06-14T00:43:21Z

@ijokarumawak I accidentally squashed the changes that @markap14 requested into your commit.

ijokarumawak · 2018-06-14T02:17:23Z

@MikeThomsen that's ok, I think it's time to squash all commits into one, I'm fine to lose my commit histories. Now we have #2777 merged into master, so we can utilize it. Can you update RestLookupService to do that? Like I did it locally here. I am +1 once this gets done.
ijokarumawak@f97924e

…ests.

1. Added 'Base URL' property to address environment specific part of URL. 2. Removed 'Record Path Property Name' property, because the name of a resulted record field of a record path can be obtained by field name. 3. Lower cased HTTP method name should be used throughout. 4. Added mimeType require check when body is specified. 5. Added debug log to print HTTP response code. 6. Prepare for NIFI-5287. 7. Fixed that mime.type being used regardless of whether body is specified or not, caused NullPointerException when 'mime.type' is not specified when it is not required. 8. Updated documentation.

…ation.

- Use PropertyValue instead of PreparedQuery to utilize Variable Registry. - Removed BASE_URL because Variable Registry can be used at URL

MikeThomsen · 2018-06-14T11:01:15Z

@ijokarumawak Added a new commit that should get it to close out.

ijokarumawak · 2018-06-15T06:38:42Z

@MikeThomsen I squashed all commits into one, did manual test. LGTM, +1. merging to master! Thank you!

MikeThomsen force-pushed the NIFI-5214 branch from 0a076cc to 71e52da Compare May 20, 2018 18:22

MikeThomsen mentioned this pull request May 20, 2018

NIFI-4199: Consistent proxy support across components #2704

Closed

11 tasks

MikeThomsen force-pushed the NIFI-5214 branch from 71e52da to d127324 Compare May 20, 2018 22:02

ottobackwards reviewed May 21, 2018

View reviewed changes

ijokarumawak reviewed May 22, 2018

View reviewed changes

MikeThomsen force-pushed the NIFI-5214 branch from d71b094 to 566994e Compare June 1, 2018 19:04

markap14 requested changes Jun 1, 2018

View reviewed changes

ijokarumawak reviewed Jun 4, 2018

View reviewed changes

ottobackwards reviewed Jun 4, 2018

View reviewed changes

ijokarumawak reviewed Jun 5, 2018

View reviewed changes

ijokarumawak requested changes Jun 6, 2018

View reviewed changes

ijokarumawak mentioned this pull request Jun 12, 2018

NIFI-5214: Refactor RestLookupService. MikeThomsen/nifi#1

Merged

MikeThomsen force-pushed the NIFI-5214 branch from 51d2d92 to 8829e53 Compare June 12, 2018 12:10

MikeThomsen and others added 22 commits June 14, 2018 06:47

NIFI-5214 Added REST LookupService

7724e06

NIFI-5214 Added support for the new ProxyConfigurationService

8584d17

NIFI-5214 Integration tests added.

b1fcac9

NIFI-5214 Added missing pom.xml and a change from a code review.

f4d3493

NIFI-5214 Added another tag based on code review.

f8f5138

NIFI-5214 Added user-defined header support.

f694f67

NIFI-5214 Added Basic Auth support.

968a0c0

NIFI-5214 Moved documentation.

a03f409

NIFI-5214 Fixed checkstyle issues; added changes requested in a review.

95bbe3f

NIFI-5214 Added changes requested in a code review.

a7c6e22

NIFI-5214 Added verb test and @dynamicproperties

b78d00a

NIFI-5214 Added templated URL support to RestLookupService.

6c18a59

NIFI-5214 Fixed documentation based on code review changes.

d7b09ea

NIFI-5214 Changed RestLookupService to use a property descriptor.

c2c606c

NIFI-5214 Updated documentation.

155f31c

NIFI-5214 Made changes requested in code review.

51953df

NIFI-5214 Renamed nifi-standard-web-utils to reflect that it is for t…

5384129

…ests.

NIFI-5214 Fixed AWS processors that broke w/ change to TestServer loc…

0be0cca

…ation.

NIFI-5214 Added changes requested in a code review.

1687b5e

Refactored the way to evaluate EL for URL property

0853d37

- Use PropertyValue instead of PreparedQuery to utilize Variable Registry. - Removed BASE_URL because Variable Registry can be used at URL

NIFI-5214 Rebased to use new LookupService method.

1919d2e

MikeThomsen force-pushed the NIFI-5214 branch from 2ea76ef to 1919d2e Compare June 14, 2018 10:58

asfgit closed this in be63378 Jun 15, 2018

MikeThomsen deleted the NIFI-5214 branch August 14, 2024 21:14


		Record record = handleResponse(is, coordinates);

		return record != null

		@@ -277,6 +280,20 @@ private void setProxy(OkHttpClient.Builder builder) {
		}
		}

NIFI-5214 Added REST LookupService #2723

NIFI-5214 Added REST LookupService #2723

Conversation

MikeThomsen commented May 19, 2018 • edited Loading

For all changes:

For code changes:

For documentation related changes:

Note:

ottobackwards commented May 19, 2018

MikeThomsen commented May 19, 2018

ottobackwards commented May 19, 2018

MikeThomsen commented May 20, 2018

MikeThomsen commented May 20, 2018

MikeThomsen commented May 20, 2018

ottobackwards left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ottobackwards commented May 21, 2018

ottobackwards commented May 21, 2018

MikeThomsen commented May 21, 2018

ottobackwards commented May 21, 2018

ijokarumawak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeThomsen commented Jun 1, 2018

MikeThomsen commented Jun 1, 2018

markap14 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeThomsen commented Jun 2, 2018

ijokarumawak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeThomsen commented Jun 4, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ijokarumawak Jun 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ijokarumawak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MikeThomsen commented May 19, 2018 •

edited

Loading

ijokarumawak Jun 5, 2018 •

edited

Loading