Details

    • Type: Improvement
    • Status: Open
    • Priority: Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      The nifi-aws-bundle currently contains the interface for the AWSCredentialsProviderService as well as the service implementation, and dependent abstract classes and processor classes.

      This results in the following warning logged as NiFi loads:

      org.apache.nifi.nar.ExtensionManager Component org.apache.nifi.processors.aws.s3.PutS3Object is bundled with its referenced Controller Service APIs org.apache.nifi.processors.aws.credentials.provider.service.AWSCredentialsProviderService. The service APIs should not be bundled with component implementations that reference it.

      Some discussion of this issue and potential solutions occurred on the dev list.

      We also need a migration plan in addition to the new structure.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user christophercurrie opened a pull request:

          https://github.com/apache/nifi/pull/2140

          NIFI-3950 Refactor AWS bundle

          This PR provides a separate NAR for the `AWSCredentialsProviderService` API, and a new `nifi-aws-abstract-processors` jar so that custom processors can take advantage of the abstract base classes without importing duplicates of the existing implementations.

          The JIRA ticket mentions the need for a transition plan; I'm not sure what is required there, but happy to help provide whatever is needed.

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

          $ git pull https://github.com/christophercurrie/nifi NIFI-3950-aws-refactor

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

          https://github.com/apache/nifi/pull/2140.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 #2140


          commit 1c30e78716dd9f592cdbe9abc583053cb7fcd31d
          Author: Christopher Currie <christopher@currie.com>
          Date: 2017-09-10T20:19:45Z

          NIFI-3950 Refactor AWS bundle


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user christophercurrie opened a pull request: https://github.com/apache/nifi/pull/2140 NIFI-3950 Refactor AWS bundle This PR provides a separate NAR for the `AWSCredentialsProviderService` API, and a new `nifi-aws-abstract-processors` jar so that custom processors can take advantage of the abstract base classes without importing duplicates of the existing implementations. The JIRA ticket mentions the need for a transition plan; I'm not sure what is required there, but happy to help provide whatever is needed. You can merge this pull request into a Git repository by running: $ git pull https://github.com/christophercurrie/nifi NIFI-3950 -aws-refactor Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/2140.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 #2140 commit 1c30e78716dd9f592cdbe9abc583053cb7fcd31d Author: Christopher Currie <christopher@currie.com> Date: 2017-09-10T20:19:45Z NIFI-3950 Refactor AWS bundle
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user christophercurrie commented on the issue:

          https://github.com/apache/nifi/pull/2140

          The failed CI builds seem to be failing for other PRs and do not appear to be caused by this change.

          Show
          githubbot ASF GitHub Bot added a comment - Github user christophercurrie commented on the issue: https://github.com/apache/nifi/pull/2140 The failed CI builds seem to be failing for other PRs and do not appear to be caused by this change.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jvwing commented on the issue:

          https://github.com/apache/nifi/pull/2140

          Reviewing... I agree the CI failure may not be related to this change. `mvn clean install -Pcontrib-check` worked OK for me.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jvwing commented on the issue: https://github.com/apache/nifi/pull/2140 Reviewing... I agree the CI failure may not be related to this change. `mvn clean install -Pcontrib-check` worked OK for me.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jvwing commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/2140#discussion_r138251087

          — Diff: pom.xml —
          @@ -1109,12 +1109,6 @@
          </dependency>
          <dependency>
          <groupId>org.apache.nifi</groupId>

          • <artifactId>nifi-kudu-nar</artifactId>
              • End diff –

          Did you intend to remove nifi-kudu-nar?

          Show
          githubbot ASF GitHub Bot added a comment - Github user jvwing commented on a diff in the pull request: https://github.com/apache/nifi/pull/2140#discussion_r138251087 — Diff: pom.xml — @@ -1109,12 +1109,6 @@ </dependency> <dependency> <groupId>org.apache.nifi</groupId> <artifactId>nifi-kudu-nar</artifactId> End diff – Did you intend to remove nifi-kudu-nar?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jvwing commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/2140#discussion_r138251749

          — Diff: nifi-nar-bundles/nifi-aws-bundle/nifi-aws-nar/pom.xml —
          @@ -32,13 +32,14 @@
          <dependencies>
          <dependency>
          <groupId>org.apache.nifi</groupId>

          • <artifactId>nifi-standard-services-api-nar</artifactId>
            + <artifactId>nifi-aws-service-api-nar</artifactId>
            + <version>$ {project.version}

            </version>
            <type>nar</type>
            </dependency>
            <dependency>
            <groupId>org.apache.nifi</groupId>
            <artifactId>nifi-aws-processors</artifactId>

          • <version>1.4.0-SNAPSHOT</version>
              • End diff –

          I'm not well informed on the pros and cons of using the project.version variable as opposed to the literal version number. The NiFi project as a whole manages literal version numbers with the Maven Release Plugin , so the variables are not necessary to keep references consistent. What has been your experience?

          Show
          githubbot ASF GitHub Bot added a comment - Github user jvwing commented on a diff in the pull request: https://github.com/apache/nifi/pull/2140#discussion_r138251749 — Diff: nifi-nar-bundles/nifi-aws-bundle/nifi-aws-nar/pom.xml — @@ -32,13 +32,14 @@ <dependencies> <dependency> <groupId>org.apache.nifi</groupId> <artifactId>nifi-standard-services-api-nar</artifactId> + <artifactId>nifi-aws-service-api-nar</artifactId> + <version>$ {project.version} </version> <type>nar</type> </dependency> <dependency> <groupId>org.apache.nifi</groupId> <artifactId>nifi-aws-processors</artifactId> <version>1.4.0-SNAPSHOT</version> End diff – I'm not well informed on the pros and cons of using the project.version variable as opposed to the literal version number. The NiFi project as a whole manages literal version numbers with the Maven Release Plugin , so the variables are not necessary to keep references consistent. What has been your experience?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jvwing commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/2140#discussion_r138251181

          — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/pom.xml —
          @@ -253,11 +253,6 @@
          </dependency>
          <dependency>
          <groupId>org.apache.nifi</groupId>

          • <artifactId>nifi-schema-registry-service-api</artifactId>
              • End diff –

          Is nifi-schema-registry-service-api intentionally removed here?

          Show
          githubbot ASF GitHub Bot added a comment - Github user jvwing commented on a diff in the pull request: https://github.com/apache/nifi/pull/2140#discussion_r138251181 — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/pom.xml — @@ -253,11 +253,6 @@ </dependency> <dependency> <groupId>org.apache.nifi</groupId> <artifactId>nifi-schema-registry-service-api</artifactId> End diff – Is nifi-schema-registry-service-api intentionally removed here?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jvwing commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/2140#discussion_r138248781

          — Diff: nifi-assembly/pom.xml —
          @@ -1,13 +1,13 @@
          <?xml version="1.0" encoding="UTF-8"?>
          <!- 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
            +<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
              • End diff –

          Several of the diffs in this PR are whitespace-only changes. Are these intentional?

          Show
          githubbot ASF GitHub Bot added a comment - Github user jvwing commented on a diff in the pull request: https://github.com/apache/nifi/pull/2140#discussion_r138248781 — Diff: nifi-assembly/pom.xml — @@ -1,13 +1,13 @@ <?xml version="1.0" encoding="UTF-8"?> <! - 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 +<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor End diff – Several of the diffs in this PR are whitespace-only changes. Are these intentional?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jvwing commented on the issue:

          https://github.com/apache/nifi/pull/2140

          @christophercurrie - With respect to a transition plan, I'm not sure exactly what we need. I'll have to get back to you on that. In vague concept, users who have built custom processors and custom controller services against the existing API should have a smooth upgrade experience to the new one. I'll try to work out a more concrete definition for 'smooth'.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jvwing commented on the issue: https://github.com/apache/nifi/pull/2140 @christophercurrie - With respect to a transition plan, I'm not sure exactly what we need. I'll have to get back to you on that. In vague concept, users who have built custom processors and custom controller services against the existing API should have a smooth upgrade experience to the new one. I'll try to work out a more concrete definition for 'smooth'.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user christophercurrie commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/2140#discussion_r138257582

          — Diff: nifi-assembly/pom.xml —
          @@ -1,13 +1,13 @@
          <?xml version="1.0" encoding="UTF-8"?>
          <!- 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
            +<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
              • End diff –

          Not deliberate, no, probably a result of my editor settings; I can revert them if they are unwanted.

          Show
          githubbot ASF GitHub Bot added a comment - Github user christophercurrie commented on a diff in the pull request: https://github.com/apache/nifi/pull/2140#discussion_r138257582 — Diff: nifi-assembly/pom.xml — @@ -1,13 +1,13 @@ <?xml version="1.0" encoding="UTF-8"?> <! - 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 +<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor End diff – Not deliberate, no, probably a result of my editor settings; I can revert them if they are unwanted.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user christophercurrie commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/2140#discussion_r138257624

          — Diff: pom.xml —
          @@ -1109,12 +1109,6 @@
          </dependency>
          <dependency>
          <groupId>org.apache.nifi</groupId>

          • <artifactId>nifi-kudu-nar</artifactId>
              • End diff –

          It was duplicated in the pom; Maven issued a warning to me about it.

          Show
          githubbot ASF GitHub Bot added a comment - Github user christophercurrie commented on a diff in the pull request: https://github.com/apache/nifi/pull/2140#discussion_r138257624 — Diff: pom.xml — @@ -1109,12 +1109,6 @@ </dependency> <dependency> <groupId>org.apache.nifi</groupId> <artifactId>nifi-kudu-nar</artifactId> End diff – It was duplicated in the pom; Maven issued a warning to me about it.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user christophercurrie commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/2140#discussion_r138257661

          — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/pom.xml —
          @@ -253,11 +253,6 @@
          </dependency>
          <dependency>
          <groupId>org.apache.nifi</groupId>

          • <artifactId>nifi-schema-registry-service-api</artifactId>
              • End diff –

          It was duplicated in the pom, once as a test dep and once as a regular dep. Maven warned me about it.

          Show
          githubbot ASF GitHub Bot added a comment - Github user christophercurrie commented on a diff in the pull request: https://github.com/apache/nifi/pull/2140#discussion_r138257661 — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/pom.xml — @@ -253,11 +253,6 @@ </dependency> <dependency> <groupId>org.apache.nifi</groupId> <artifactId>nifi-schema-registry-service-api</artifactId> End diff – It was duplicated in the pom, once as a test dep and once as a regular dep. Maven warned me about it.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user christophercurrie commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/2140#discussion_r138257973

          — Diff: nifi-nar-bundles/nifi-aws-bundle/nifi-aws-nar/pom.xml —
          @@ -32,13 +32,14 @@
          <dependencies>
          <dependency>
          <groupId>org.apache.nifi</groupId>

          • <artifactId>nifi-standard-services-api-nar</artifactId>
            + <artifactId>nifi-aws-service-api-nar</artifactId>
            + <version>$ {project.version}

            </version>
            <type>nar</type>
            </dependency>
            <dependency>
            <groupId>org.apache.nifi</groupId>
            <artifactId>nifi-aws-processors</artifactId>

          • <version>1.4.0-SNAPSHOT</version>
              • End diff –

          It's a style choice, one I follow out of habit. The release plugin is perfectly happy with such references, and I find it clearer to declare the version once at the the parent pom reference and stick with it, unless the version is deliberately varying. But that's for your project to decide, so happy to revert my habit here if wanted.

          Show
          githubbot ASF GitHub Bot added a comment - Github user christophercurrie commented on a diff in the pull request: https://github.com/apache/nifi/pull/2140#discussion_r138257973 — Diff: nifi-nar-bundles/nifi-aws-bundle/nifi-aws-nar/pom.xml — @@ -32,13 +32,14 @@ <dependencies> <dependency> <groupId>org.apache.nifi</groupId> <artifactId>nifi-standard-services-api-nar</artifactId> + <artifactId>nifi-aws-service-api-nar</artifactId> + <version>$ {project.version} </version> <type>nar</type> </dependency> <dependency> <groupId>org.apache.nifi</groupId> <artifactId>nifi-aws-processors</artifactId> <version>1.4.0-SNAPSHOT</version> End diff – It's a style choice, one I follow out of habit. The release plugin is perfectly happy with such references, and I find it clearer to declare the version once at the the parent pom reference and stick with it, unless the version is deliberately varying. But that's for your project to decide, so happy to revert my habit here if wanted.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user christophercurrie commented on the issue:

          https://github.com/apache/nifi/pull/2140

          How smooth their experience will be will probably depend upon their internal project practices. Since the `nifi-aws-processors` package would have been a required dependency for such custom processors, the main change would be to require adding `nifi-aws-service-api-nar` somewhere in their nar parent chain; this change is, AFAICT, unavoidable given the nature of controller-processor decoupling.

          Show
          githubbot ASF GitHub Bot added a comment - Github user christophercurrie commented on the issue: https://github.com/apache/nifi/pull/2140 How smooth their experience will be will probably depend upon their internal project practices. Since the `nifi-aws-processors` package would have been a required dependency for such custom processors, the main change would be to require adding `nifi-aws-service-api-nar` somewhere in their nar parent chain; this change is, AFAICT, unavoidable given the nature of controller-processor decoupling.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jvwing commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/2140#discussion_r138520837

          — Diff: nifi-assembly/pom.xml —
          @@ -1,13 +1,13 @@
          <?xml version="1.0" encoding="UTF-8"?>
          <!- 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
            +<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor
              • End diff –

          I would appreciate it if you reverted them. It is easier to review a PR if the changes are focused narrowly on the subject of the ticket.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jvwing commented on a diff in the pull request: https://github.com/apache/nifi/pull/2140#discussion_r138520837 — Diff: nifi-assembly/pom.xml — @@ -1,13 +1,13 @@ <?xml version="1.0" encoding="UTF-8"?> <! - 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 +<!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor End diff – I would appreciate it if you reverted them. It is easier to review a PR if the changes are focused narrowly on the subject of the ticket.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jvwing commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/2140#discussion_r138520916

          — Diff: pom.xml —
          @@ -1109,12 +1109,6 @@
          </dependency>
          <dependency>
          <groupId>org.apache.nifi</groupId>

          • <artifactId>nifi-kudu-nar</artifactId>
              • End diff –

          I understand. I think that might make a great change in a different PR for a different ticket. But again, I would prefer not to evaluate it as part of this one.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jvwing commented on a diff in the pull request: https://github.com/apache/nifi/pull/2140#discussion_r138520916 — Diff: pom.xml — @@ -1109,12 +1109,6 @@ </dependency> <dependency> <groupId>org.apache.nifi</groupId> <artifactId>nifi-kudu-nar</artifactId> End diff – I understand. I think that might make a great change in a different PR for a different ticket. But again, I would prefer not to evaluate it as part of this one.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jvwing commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/2140#discussion_r138521026

          — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/pom.xml —
          @@ -253,11 +253,6 @@
          </dependency>
          <dependency>
          <groupId>org.apache.nifi</groupId>

          • <artifactId>nifi-schema-registry-service-api</artifactId>
              • End diff –

          Again, a great change in a different PR for a different ticket.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jvwing commented on a diff in the pull request: https://github.com/apache/nifi/pull/2140#discussion_r138521026 — Diff: nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/pom.xml — @@ -253,11 +253,6 @@ </dependency> <dependency> <groupId>org.apache.nifi</groupId> <artifactId>nifi-schema-registry-service-api</artifactId> End diff – Again, a great change in a different PR for a different ticket.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user christophercurrie commented on the issue:

          https://github.com/apache/nifi/pull/2140

          Not a problem on the reverts. The original instructions asked for a squashed commit. Should I create a new smaller PR, or add the revert commit to this one?

          Show
          githubbot ASF GitHub Bot added a comment - Github user christophercurrie commented on the issue: https://github.com/apache/nifi/pull/2140 Not a problem on the reverts. The original instructions asked for a squashed commit. Should I create a new smaller PR, or add the revert commit to this one?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jvwing commented on the issue:

          https://github.com/apache/nifi/pull/2140

          Please add the commits to this one. It usually helps reviewing to see the commits separately, and it's easy enough to squash at the end.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jvwing commented on the issue: https://github.com/apache/nifi/pull/2140 Please add the commits to this one. It usually helps reviewing to see the commits separately, and it's easy enough to squash at the end.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user bbende commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/2140#discussion_r138704486

          — Diff: nifi-nar-bundles/nifi-aws-bundle/nifi-aws-nar/pom.xml —
          @@ -32,13 +32,14 @@
          <dependencies>
          <dependency>
          <groupId>org.apache.nifi</groupId>

          • <artifactId>nifi-standard-services-api-nar</artifactId>
            + <artifactId>nifi-aws-service-api-nar</artifactId>
            + <version>$ {project.version}

            </version>
            <type>nar</type>
            </dependency>
            <dependency>
            <groupId>org.apache.nifi</groupId>
            <artifactId>nifi-aws-processors</artifactId>

          • <version>1.4.0-SNAPSHOT</version>
              • End diff –

          We actually had a problem in the past with the release plugin and project.version, I can't remember the specifics and it may very well not be an issue anymore, but we did specifically make the choice to use the literal version everywhere so I'd prefer to stick with that

          Show
          githubbot ASF GitHub Bot added a comment - Github user bbende commented on a diff in the pull request: https://github.com/apache/nifi/pull/2140#discussion_r138704486 — Diff: nifi-nar-bundles/nifi-aws-bundle/nifi-aws-nar/pom.xml — @@ -32,13 +32,14 @@ <dependencies> <dependency> <groupId>org.apache.nifi</groupId> <artifactId>nifi-standard-services-api-nar</artifactId> + <artifactId>nifi-aws-service-api-nar</artifactId> + <version>$ {project.version} </version> <type>nar</type> </dependency> <dependency> <groupId>org.apache.nifi</groupId> <artifactId>nifi-aws-processors</artifactId> <version>1.4.0-SNAPSHOT</version> End diff – We actually had a problem in the past with the release plugin and project.version, I can't remember the specifics and it may very well not be an issue anymore, but we did specifically make the choice to use the literal version everywhere so I'd prefer to stick with that
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user bbende commented on the issue:

          https://github.com/apache/nifi/pull/2140

          Regarding the migration strategy... Lets make sure we document on the migration wiki page [1] that anyone who built custom components using the AWS controller service should rebuild them with the new dependency as part of their upgrade process.

          Given our component versioning, it would actually be possible to run the old stuff and the new stuff at the same time, but it would require a complicated setup. I think you'd have to leave the 1.3.0 AWS NAR, 1.3.0 standard services API NAR, and possibly 1.3.0 standard NAR, plus the users custom NAR that depended on 1.3.0 AWS NAR. Probably not the recommended approach, but a fallback option.

          [1] https://cwiki.apache.org/confluence/display/NIFI/Migration+Guidance

          Show
          githubbot ASF GitHub Bot added a comment - Github user bbende commented on the issue: https://github.com/apache/nifi/pull/2140 Regarding the migration strategy... Lets make sure we document on the migration wiki page [1] that anyone who built custom components using the AWS controller service should rebuild them with the new dependency as part of their upgrade process. Given our component versioning, it would actually be possible to run the old stuff and the new stuff at the same time, but it would require a complicated setup. I think you'd have to leave the 1.3.0 AWS NAR, 1.3.0 standard services API NAR, and possibly 1.3.0 standard NAR, plus the users custom NAR that depended on 1.3.0 AWS NAR. Probably not the recommended approach, but a fallback option. [1] https://cwiki.apache.org/confluence/display/NIFI/Migration+Guidance
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jvwing commented on the issue:

          https://github.com/apache/nifi/pull/2140

          Thanks, @bbende, I'll take up the migration guidance for the wiki. As part of reviewing this PR, I am building some sample processor and service projects to reproduce the problems and check the fix. I plan to work through the migration steps myself and can document the process.

          It's very likely that I'll have more questions for you as I get into details of it.

          Show
          githubbot ASF GitHub Bot added a comment - Github user jvwing commented on the issue: https://github.com/apache/nifi/pull/2140 Thanks, @bbende, I'll take up the migration guidance for the wiki. As part of reviewing this PR, I am building some sample processor and service projects to reproduce the problems and check the fix. I plan to work through the migration steps myself and can document the process. It's very likely that I'll have more questions for you as I get into details of it.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user christophercurrie commented on the issue:

          https://github.com/apache/nifi/pull/2140

          @jvwing I have pushed a commit that removes all unnecessary changes from the PR. Sorry for the delay.

          Show
          githubbot ASF GitHub Bot added a comment - Github user christophercurrie commented on the issue: https://github.com/apache/nifi/pull/2140 @jvwing I have pushed a commit that removes all unnecessary changes from the PR. Sorry for the delay.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jvwing commented on the issue:

          https://github.com/apache/nifi/pull/2140

          Thanks for the update, @christophercurrie . This PR is looking pretty good:

          • Passes the full suite of unit tests with contrib-check.
          • AWS processors and controller service still work OK in my testing.
          • Provides a good migration experience – just rebuild against NiFi 1.4.0 nars – better than I feared. More below.

          One thing we still need is a set of LICENSE/NOTICE files for nifi-aws-service-api-nar, similar to what is now in the nifi-aws-nar. I believe the NOTICE file can be pared down to only reference the aws-sdk.

          *Migration Experience*
          I created a [simple AWS bundle](https://github.com/jvwing/sample-aws-bundle) targeting NiFi 1.3.0, and went through the exercise of [migrating it](https://github.com/jvwing/sample-aws-bundle/tree/target-nifi-1.4.0) to 1.4.0 as of this PR. It seems "smooth" enough to me.

          • Advancing the NiFi dependency version to 1.4.0 and rebuilding is enough, maintaining the NAR dependency on `nifi-aws-nar`.
          • For bundles that only implement controller service interfaces, they may optionally change their NAR dependency to `nifi-aws-service-api-nar`. Since nifi-aws-nar already has this NAR dependency, I believe this is a recommended, but not strictly necessary step.
          Show
          githubbot ASF GitHub Bot added a comment - Github user jvwing commented on the issue: https://github.com/apache/nifi/pull/2140 Thanks for the update, @christophercurrie . This PR is looking pretty good: Passes the full suite of unit tests with contrib-check. AWS processors and controller service still work OK in my testing. Provides a good migration experience – just rebuild against NiFi 1.4.0 nars – better than I feared. More below. One thing we still need is a set of LICENSE/NOTICE files for nifi-aws-service-api-nar, similar to what is now in the nifi-aws-nar. I believe the NOTICE file can be pared down to only reference the aws-sdk. * Migration Experience * I created a [simple AWS bundle] ( https://github.com/jvwing/sample-aws-bundle ) targeting NiFi 1.3.0, and went through the exercise of [migrating it] ( https://github.com/jvwing/sample-aws-bundle/tree/target-nifi-1.4.0 ) to 1.4.0 as of this PR. It seems "smooth" enough to me. Advancing the NiFi dependency version to 1.4.0 and rebuilding is enough, maintaining the NAR dependency on `nifi-aws-nar`. For bundles that only implement controller service interfaces, they may optionally change their NAR dependency to `nifi-aws-service-api-nar`. Since nifi-aws-nar already has this NAR dependency, I believe this is a recommended, but not strictly necessary step.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user jvwing commented on the issue:

          https://github.com/apache/nifi/pull/2140

          More migration notes:

          • My 1.3.0 AWS processor worked OK without modification when used with this PR.
          • My 1.3.0 AWS controller service did not work with just the NAR file, with the expected incompatibility error:

          > AWS Credentials Provider service' validated against '8902b809-015e-1000-46c3-e321c1d9a1b4' is invalid because SampleAWSCreds - 1.3.0 from sample - sample-aws-services-nar is not compatible with AWSCredentialsProviderService - 1.4.0-SNAPSHOT from org.apache.nifi - nifi-aws-nar

          • However, if `nifi-aws-nar-1.3.0.nar` was included side-by-side with `nifi-aws-nar-1.4.0.nar`, the 1.3.0 processor worked with the 1.3.0 controller service, unchanged.

          @bbende , I'm not sure the last option is what you described above. I was expecting to add more NARs. But it seemed plausible that the monolithic nature of nifi-aws-nar-1.3.0.nar might make it easier to dump in side-by-side?

          Show
          githubbot ASF GitHub Bot added a comment - Github user jvwing commented on the issue: https://github.com/apache/nifi/pull/2140 More migration notes: My 1.3.0 AWS processor worked OK without modification when used with this PR. My 1.3.0 AWS controller service did not work with just the NAR file, with the expected incompatibility error: > AWS Credentials Provider service' validated against '8902b809-015e-1000-46c3-e321c1d9a1b4' is invalid because SampleAWSCreds - 1.3.0 from sample - sample-aws-services-nar is not compatible with AWSCredentialsProviderService - 1.4.0-SNAPSHOT from org.apache.nifi - nifi-aws-nar However, if `nifi-aws-nar-1.3.0.nar` was included side-by-side with `nifi-aws-nar-1.4.0.nar`, the 1.3.0 processor worked with the 1.3.0 controller service, unchanged. @bbende , I'm not sure the last option is what you described above. I was expecting to add more NARs. But it seemed plausible that the monolithic nature of nifi-aws-nar-1.3.0.nar might make it easier to dump in side-by-side?

            People

            • Assignee:
              Unassigned
              Reporter:
              jameswing James Wing
            • Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:

                Development