[GitHub] nifi-minifi pull request #108: MINIFI-415 Adjusting logging when a bundle is...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/nifi-minifi/pull/108


---

[GitHub] nifi-minifi pull request #108: MINIFI-415 Adjusting logging when a bundle is...

[apiri]

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

https://github.com/apache/nifi-minifi/pull/108#discussion_r161253393
  
--- Diff: 
minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-runtime/src/main/java/org/apache/nifi/minifi/FlowEnricher.java
 ---
@@ -130,17 +127,13 @@ private void enrichComponent(EnrichingElementAdapter 
componentToEnrich, Map componentToEnrichBundleVersions = 
componentToEnrichVersionToBundles.values().stream()
 .map(bundle -> 
bundle.getBundleDetails().getCoordinate().getVersion()).collect(Collectors.toSet());
-final String componentToEnrichId = 
componentToEnrich.getComponentId();
-String bundleVersion = 
componentToEnrichBundleVersions.stream().sorted().reduce((version, 
otherVersion) -> otherVersion).orElse(null);
-if (bundleVersion != null) {
-
componentToEnrich.setBundleInformation(componentToEnrichVersionToBundles.get(bundleVersion).getBundleDetails().getCoordinate());
-}
-logger.info("Enriching {} with bundle {}", new Object[]{});
-
+final String bundleVersion = 
componentToEnrichBundleVersions.stream().sorted().reduce((version, 
otherVersion) -> otherVersion).orElse(null);
+final BundleCoordinate enrichingCoordinate = 
componentToEnrichVersionToBundles.get(bundleVersion).getBundleDetails().getCoordinate();
--- End diff --

Okay, fair enough.  Definitely looks odd and I will adjust that.


---

[GitHub] nifi-minifi pull request #108: MINIFI-415 Adjusting logging when a bundle is...

[jzonthemtn]

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

https://github.com/apache/nifi-minifi/pull/108#discussion_r161116290
  
--- Diff: 
minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-runtime/src/main/java/org/apache/nifi/minifi/FlowEnricher.java
 ---
@@ -130,17 +127,13 @@ private void enrichComponent(EnrichingElementAdapter 
componentToEnrich, Map componentToEnrichBundleVersions = 
componentToEnrichVersionToBundles.values().stream()
 .map(bundle -> 
bundle.getBundleDetails().getCoordinate().getVersion()).collect(Collectors.toSet());
-final String componentToEnrichId = 
componentToEnrich.getComponentId();
-String bundleVersion = 
componentToEnrichBundleVersions.stream().sorted().reduce((version, 
otherVersion) -> otherVersion).orElse(null);
-if (bundleVersion != null) {
-
componentToEnrich.setBundleInformation(componentToEnrichVersionToBundles.get(bundleVersion).getBundleDetails().getCoordinate());
-}
-logger.info("Enriching {} with bundle {}", new Object[]{});
-
+final String bundleVersion = 
componentToEnrichBundleVersions.stream().sorted().reduce((version, 
otherVersion) -> otherVersion).orElse(null);
+final BundleCoordinate enrichingCoordinate = 
componentToEnrichVersionToBundles.get(bundleVersion).getBundleDetails().getCoordinate();
--- End diff --

As long as its guaranteed to not be `null` I think it is ok as is. The 
`orElse` is what raised a flag for me. 


---

[GitHub] nifi-minifi pull request #108: MINIFI-415 Adjusting logging when a bundle is...

[apiri]

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

https://github.com/apache/nifi-minifi/pull/108#discussion_r161095169
  
--- Diff: 
minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-runtime/src/main/java/org/apache/nifi/minifi/FlowEnricher.java
 ---
@@ -130,17 +127,13 @@ private void enrichComponent(EnrichingElementAdapter 
componentToEnrich, Map componentToEnrichBundleVersions = 
componentToEnrichVersionToBundles.values().stream()
 .map(bundle -> 
bundle.getBundleDetails().getCoordinate().getVersion()).collect(Collectors.toSet());
-final String componentToEnrichId = 
componentToEnrich.getComponentId();
-String bundleVersion = 
componentToEnrichBundleVersions.stream().sorted().reduce((version, 
otherVersion) -> otherVersion).orElse(null);
-if (bundleVersion != null) {
-
componentToEnrich.setBundleInformation(componentToEnrichVersionToBundles.get(bundleVersion).getBundleDetails().getCoordinate());
-}
-logger.info("Enriching {} with bundle {}", new Object[]{});
-
+final String bundleVersion = 
componentToEnrichBundleVersions.stream().sorted().reduce((version, 
otherVersion) -> otherVersion).orElse(null);
+final BundleCoordinate enrichingCoordinate = 
componentToEnrichVersionToBundles.get(bundleVersion).getBundleDetails().getCoordinate();
--- End diff --

Probably doesn't hurt but my thought when I looked at it again was that 
given that we have already determined that there is a non-empty collection of 
possible candidates and we are just selecting the greatest of these through the 
reduction, there will be a guaranteed value.  What makes it awkward is the 
orElse that was left behind.  I suppose I could change this to just #get() and 
the meaning would be a little clearer.  What do you think makes the most sense?


---

[GitHub] nifi-minifi pull request #108: MINIFI-415 Adjusting logging when a bundle is...

[jzonthemtn]

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

https://github.com/apache/nifi-minifi/pull/108#discussion_r161082784
  
--- Diff: 
minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-runtime/src/main/java/org/apache/nifi/minifi/FlowEnricher.java
 ---
@@ -130,17 +127,13 @@ private void enrichComponent(EnrichingElementAdapter 
componentToEnrich, Map componentToEnrichBundleVersions = 
componentToEnrichVersionToBundles.values().stream()
 .map(bundle -> 
bundle.getBundleDetails().getCoordinate().getVersion()).collect(Collectors.toSet());
-final String componentToEnrichId = 
componentToEnrich.getComponentId();
-String bundleVersion = 
componentToEnrichBundleVersions.stream().sorted().reduce((version, 
otherVersion) -> otherVersion).orElse(null);
-if (bundleVersion != null) {
-
componentToEnrich.setBundleInformation(componentToEnrichVersionToBundles.get(bundleVersion).getBundleDetails().getCoordinate());
-}
-logger.info("Enriching {} with bundle {}", new Object[]{});
-
+final String bundleVersion = 
componentToEnrichBundleVersions.stream().sorted().reduce((version, 
otherVersion) -> otherVersion).orElse(null);
+final BundleCoordinate enrichingCoordinate = 
componentToEnrichVersionToBundles.get(bundleVersion).getBundleDetails().getCoordinate();
--- End diff --

Is the null check on `bundleVersion` still necessary to prevent trying to 
get a map value by key `null`?


---

[GitHub] nifi-minifi pull request #108: MINIFI-415 Adjusting logging when a bundle is...

[apiri]

GitHub user apiri opened a pull request:

https://github.com/apache/nifi-minifi/pull/108

MINIFI-415 Adjusting logging when a bundle is automatically selected

MINIFI-415 Adjusting logging when a bundle is automatically selected for a 
component when multiple options are available.

Thank you for submitting a contribution to Apache NiFi - MiNiFi.

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 MINIFI- where  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-minifi 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](http://www.apache.org/legal/resolved.html#category-a)? 
- [ ] If applicable, have you updated the LICENSE file, including the main 
LICENSE file under minifi-assembly?
- [ ] If applicable, have you updated the NOTICE file, including the main 
NOTICE file found under minifi-assembly?

### 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.


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

$ git pull https://github.com/apiri/nifi-minifi MINIFI-415

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

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


commit 3797f8c1c09e331b544fb85d7cc0cc5668cf13b4
Author: Aldrin Piri 
Date:   2018-01-11T18:56:54Z

MINIFI-415 Adjusting logging when a bundle is automatically selected for a 
component when multiple options are available.




---