You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by pvillard31 <gi...@git.apache.org> on 2018/03/21 21:30:27 UTC
[GitHub] nifi pull request #2575: NIFI-4809 - Implement a SiteToSiteMetricsReportingT...
GitHub user pvillard31 opened a pull request:
https://github.com/apache/nifi/pull/2575
NIFI-4809 - Implement a SiteToSiteMetricsReportingTask
This PR is an update of the previous one (#2430) that has been reverted to fix a dependency issue.
I've manually added a JSON Record reader in the abstract class using the code of the existing reader. For now, it's only used by the Metrics reporting task, but I plan to use the same approach for the other S2S reporting tasks.
**Do not merge this PR as-is**.
I currently see two issues:
- the one already reported by @mattyb149: the record writer is not showing the reporting task in the "referencing components". I think it's probably a framework issue that could be solved in a separate JIRA. Will try to have a look later.
- more importantly, I see an issue when using a writer with an access schema strategy using the Avro Schema registry CS. In that case, I get a Schema not found exception "Failed to write metrics using record writer: Unable to find schema with name 'metrics'". Not sure what is causing it, will have a look asap. If using "inherit record schema" strategy, it works as expected.
Wanted to open this PR, to have feedbacks on the approach regarding the JSON record reader.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/pvillard31/nifi NIFI-4809
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/2575.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 #2575
----
commit db16a1617836dd02826605604b212d05190b5267
Author: Pierre Villard <pi...@...>
Date: 2018-01-23T22:15:18Z
NIFI-4809 - Implement a SiteToSiteMetricsReportingTask
commit 227b410f1edb9198a8ebd2b4b5335fd34e558623
Author: Pierre Villard <pi...@...>
Date: 2018-03-21T20:35:29Z
Fixed dependency issue by providing a local JSON reader
----
---
[GitHub] nifi issue #2575: NIFI-4809 - Implement a SiteToSiteMetricsReportingTask
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the issue:
https://github.com/apache/nifi/pull/2575
Both issues seem to be resolved on master, I have rebased against master.
---
[GitHub] nifi pull request #2575: NIFI-4809 - Implement a SiteToSiteMetricsReportingT...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/2575
---
[GitHub] nifi issue #2575: NIFI-4809 - Implement a SiteToSiteMetricsReportingTask
Posted by mattyb149 <gi...@git.apache.org>.
Github user mattyb149 commented on the issue:
https://github.com/apache/nifi/pull/2575
I saw the "unable to find schema" issue in the last PR too, it seems to be related to the onPropertyModified() method of AvroSchemaRegistry, that's where the actual schemas get populated, but while debugging I saw that the name was in the schema map but there was no actual schema stored. Not sure if this is a lifecycle order-of-operations thing or not, but when I deleted the schema from the registry, saved it, then added it back, it worked. Just some extra info while you're pathfinding :)
---
[GitHub] nifi issue #2575: NIFI-4809 - Implement a SiteToSiteMetricsReportingTask
Posted by pvillard31 <gi...@git.apache.org>.
Github user pvillard31 commented on the issue:
https://github.com/apache/nifi/pull/2575
Done, thanks @mattyb149 !
---
[GitHub] nifi issue #2575: NIFI-4809 - Implement a SiteToSiteMetricsReportingTask
Posted by mattyb149 <gi...@git.apache.org>.
Github user mattyb149 commented on the issue:
https://github.com/apache/nifi/pull/2575
Mind doing another rebase? I will review shortly thereafter, thanks!
---
[GitHub] nifi pull request #2575: NIFI-4809 - Implement a SiteToSiteMetricsReportingT...
Posted by mattyb149 <gi...@git.apache.org>.
Github user mattyb149 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2575#discussion_r180769501
--- Diff: nifi-nar-bundles/nifi-ambari-bundle/nifi-ambari-reporting-task/pom.xml ---
@@ -53,6 +43,11 @@
<artifactId>nifi-utils</artifactId>
<version>1.6.0-SNAPSHOT</version>
</dependency>
+ <dependency>
+ <groupId>org.apache.nifi</groupId>
+ <artifactId>nifi-reporting-utils</artifactId>
+ <version>1.6.0-SNAPSHOT</version>
--- End diff --
You'll need to update these after rebase
---
[GitHub] nifi issue #2575: NIFI-4809 - Implement a SiteToSiteMetricsReportingTask
Posted by mattyb149 <gi...@git.apache.org>.
Github user mattyb149 commented on the issue:
https://github.com/apache/nifi/pull/2575
+1 LGTM, ran full build with unit tests and tried Ambari Format as well as Record Format with an AvroRecordSetWriter. Verified the records are in the prescribed format and the standard S2S reporting task attributes are correct. Also the documentation is great, thanks for this addition! Merging to master
---