Skip to content

NIFI-1690 Changed MonitorMemory to use allowable values for pool names#328

Closed
olegz wants to merge 1 commit intoapache:masterfrom
olegz:NIFI-1690
Closed

NIFI-1690 Changed MonitorMemory to use allowable values for pool names#328
olegz wants to merge 1 commit intoapache:masterfrom
olegz:NIFI-1690

Conversation

@olegz
Copy link
Contributor

@olegz olegz commented Apr 5, 2016

  • removed dead code from MonitorMemory
  • added MonitorMemoryTest
  • minor refactoring in MonitorMemory
  • initial fix for NIFI-1731 (WARN/INFO logging) that was required by MonitorMemoryTest

@olegz olegz force-pushed the NIFI-1690 branch 5 times, most recently from d8f7efd to a4bcaeb Compare April 6, 2016 13:31
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
.defaultValue(null)
.allowableValues(memPoolAllowableValues)
.defaultValue(memPoolAllowableValues.length == 0 ? null : memPoolAllowableValues[0].getValue())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're better off simply not having a default value.

@olegz
Copy link
Contributor Author

olegz commented Jun 15, 2016

@alopresto all is addressed. Let me know.

@alopresto
Copy link
Contributor

Thanks @olegz . If you rebase and squash, I'll verify and merge.

@olegz
Copy link
Contributor Author

olegz commented Jun 15, 2016

@alopresto all done

@alopresto
Copy link
Contributor

I've tried to build this locally twice (clean checkouts and everything). It is failing with a compilation error in MonitorMemoryTest saying that UserService cannot be found. I had to clean up a couple checkstyle violations in nifi-web-api to get to that point. Those fixes are available here.

The compilation error is below:

[INFO] --- maven-compiler-plugin:3.2:testCompile (default-testCompile) @ nifi-standard-reporting-tasks ---
[INFO] Changes detected - recompiling the module!
[INFO] Compiling 3 source files to /Users/alopresto/Workspace/nifi/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/target/test-classes
[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] /Users/alopresto/Workspace/nifi/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/test/java/org/apache/nifi/controller/MonitorMemoryTest.java:[28,36] error: cannot find symbol
[ERROR]   symbol:   class UserService
  location: package org.apache.nifi.admin.service
/Users/alopresto/Workspace/nifi/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/test/java/org/apache/nifi/controller/MonitorMemoryTest.java:[136,21] error: cannot find symbol
[INFO] 2 errors

Any thoughts?

- removed dead code from MonitorMemory
- added MonitorMemoryTest
- minor refactoring in MonitorMemory
- initial fix for NIFI-1731 (WARN/INFO logging) that was required by MonitorMemoryTest

NIFI-1690 polishing

NIFI-1690 address PR comments, removed default value for MEMORY_POOL_PROPERTY

NIFI-1690 addressed latest PR comments

NIFI-1690 fixed breaking changes
@olegz
Copy link
Contributor Author

olegz commented Jun 16, 2016

@alopresto just fixed it. Took me a while to figure out what's going on but you can read some details here: https://issues.apache.org/jira/browse/NIFI-1730 (last comment). Also, I am now going to have to make a separate PR for 0.x. Arghhhhhhh

@asfgit asfgit closed this in 8e4a453 Jun 16, 2016
@alopresto
Copy link
Contributor

Merged and closed. Will do the same for PR 533.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants