You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by bbende <gi...@git.apache.org> on 2018/05/09 20:57:13 UTC

[GitHub] nifi-registry pull request #117: NIFIREG-160 Implement a hook provider

GitHub user bbende opened a pull request:

    https://github.com/apache/nifi-registry/pull/117

    NIFIREG-160 Implement a hook provider

    For whoever reviews/merges this, please keep the commit history and don't squash.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/bbende/nifi-registry hook-provider

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi-registry/pull/117.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #117
    
----
commit 3371a07068dbbd5b3984f386cf61bc297ba608a9
Author: Pierre Villard <pi...@...>
Date:   2018-04-06T14:58:33Z

    NIFIREG-160 - Initial hook provider

commit f99cf19d49bf5d301ada83a5c128b645da00458f
Author: Bryan Bende <bb...@...>
Date:   2018-05-08T17:38:33Z

    NIFIREG-160 - Making event hooks asynchronous

----


---

[GitHub] nifi-registry issue #117: NIFIREG-160 Implement a hook provider

Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:

    https://github.com/apache/nifi-registry/pull/117
  
    For an easy way to test this you can turn uncomment the LoggingEventHookProvider in providers.xml and then use the registry as normal to create buckets and save flows from NiFi, then tail or inspect logs/nifi-registry-event.log


---

[GitHub] nifi-registry issue #117: NIFIREG-160 Implement a hook provider

Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the issue:

    https://github.com/apache/nifi-registry/pull/117
  
    Hi @bbende - I should be able to review and test shortly, will let you know. Thanks again for this PR!


---

[GitHub] nifi-registry pull request #117: NIFIREG-160 Implement a hook provider

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/nifi-registry/pull/117


---

[GitHub] nifi-registry pull request #117: NIFIREG-160 Implement a hook provider

Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:

    https://github.com/apache/nifi-registry/pull/117#discussion_r188356509
  
    --- Diff: nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/hook/TestScriptEventHookProvider.java ---
    @@ -0,0 +1,45 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.nifi.registry.provider.hook;
    +
    +import static org.mockito.ArgumentMatchers.any;
    +import static org.mockito.Mockito.when;
    +
    +import org.apache.nifi.registry.extension.ExtensionManager;
    +import org.apache.nifi.registry.properties.NiFiRegistryProperties;
    +import org.apache.nifi.registry.provider.ProviderCreationException;
    +import org.apache.nifi.registry.provider.ProviderFactory;
    +import org.apache.nifi.registry.provider.StandardProviderFactory;
    +import org.junit.Test;
    +import org.mockito.Mockito;
    +
    +public class TestScriptEventHookProvider {
    +
    +    @Test(expected = ProviderCreationException.class)
    +    public void testBadScriptProvider() {
    --- End diff --
    
    ok nevermind, since we expect it to fail, it should also fail on Windows


---

[GitHub] nifi-registry issue #117: NIFIREG-160 Implement a hook provider

Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the issue:

    https://github.com/apache/nifi-registry/pull/117
  
    Merged to master, thanks!


---

[GitHub] nifi-registry issue #117: NIFIREG-160 Implement a hook provider

Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:

    https://github.com/apache/nifi-registry/pull/117
  
    @pvillard31 this should be good to go pending any review feedback, let me know if you have any cycles to take a look, thanks


---

[GitHub] nifi-registry pull request #117: NIFIREG-160 Implement a hook provider

Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:

    https://github.com/apache/nifi-registry/pull/117#discussion_r188356638
  
    --- Diff: nifi-registry-framework/src/test/java/org/apache/nifi/registry/service/TestRegistryService.java ---
    @@ -81,6 +81,7 @@
         public void setup() {
             metadataService = mock(MetadataService.class);
             flowPersistenceProvider = mock(FlowPersistenceProvider.class);
    +        // eventHookProvider = mock(EventHookProvider.class);
    --- End diff --
    
    To be removed?


---

[GitHub] nifi-registry pull request #117: NIFIREG-160 Implement a hook provider

Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:

    https://github.com/apache/nifi-registry/pull/117#discussion_r188357732
  
    --- Diff: pom.xml ---
    @@ -110,8 +110,8 @@
             <jax.rs.api.version>2.1</jax.rs.api.version>
             <jersey.version>2.26</jersey.version>
             <jackson.version>2.9.2</jackson.version>
    -        <spring.boot.version>2.0.0.M7</spring.boot.version>
    -        <spring.security.version>5.0.0.RELEASE</spring.security.version>
    +        <spring.boot.version>2.0.2.RELEASE</spring.boot.version>
    --- End diff --
    
    It seems we are also specifying the versions in the notice files. Should be updated as well.


---

[GitHub] nifi-registry pull request #117: NIFIREG-160 Implement a hook provider

Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:

    https://github.com/apache/nifi-registry/pull/117#discussion_r188350335
  
    --- Diff: nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/flow/StandardFlowSnapshotContext.java ---
    @@ -33,8 +33,8 @@
         private final String flowName;
         private final int version;
         private final String comments;
    -    private final long snapshotTimestamp;
    --- End diff --
    
    Really nitpick but I think the changes on this class seem unnecessary, no?


---

[GitHub] nifi-registry issue #117: NIFIREG-160 Implement a hook provider

Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the issue:

    https://github.com/apache/nifi-registry/pull/117
  
    @bbende  I just did some tests with various possible configurations and different hooks, it's working as expected. The dedicated log file is really great and useful. Let me know if you want to do something regarding my comments or I can take care of it while merging if you are OK with it.


---

[GitHub] nifi-registry issue #117: NIFIREG-160 Implement a hook provider

Posted by bbende <gi...@git.apache.org>.
Github user bbende commented on the issue:

    https://github.com/apache/nifi-registry/pull/117
  
    @pvillard31 thanks for the review, i'm fine with you doing the minor cleanup on merge, thanks!


---

[GitHub] nifi-registry pull request #117: NIFIREG-160 Implement a hook provider

Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on a diff in the pull request:

    https://github.com/apache/nifi-registry/pull/117#discussion_r188356239
  
    --- Diff: nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/hook/TestScriptEventHookProvider.java ---
    @@ -0,0 +1,45 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one or more
    + * contributor license agreements.  See the NOTICE file distributed with
    + * this work for additional information regarding copyright ownership.
    + * The ASF licenses this file to You under the Apache License, Version 2.0
    + * (the "License"); you may not use this file except in compliance with
    + * the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package org.apache.nifi.registry.provider.hook;
    +
    +import static org.mockito.ArgumentMatchers.any;
    +import static org.mockito.Mockito.when;
    +
    +import org.apache.nifi.registry.extension.ExtensionManager;
    +import org.apache.nifi.registry.properties.NiFiRegistryProperties;
    +import org.apache.nifi.registry.provider.ProviderCreationException;
    +import org.apache.nifi.registry.provider.ProviderFactory;
    +import org.apache.nifi.registry.provider.StandardProviderFactory;
    +import org.junit.Test;
    +import org.mockito.Mockito;
    +
    +public class TestScriptEventHookProvider {
    +
    +    @Test(expected = ProviderCreationException.class)
    +    public void testBadScriptProvider() {
    --- End diff --
    
    can't test on my side but is it going to work for a build on Windows?


---