> On July 24, 2015, 6:01 p.m., Navina Ramesh wrote: > > Thanks for picking this up! It feels good to look at a refactored code. > > > > One suggestion: Please run all the intergration test (including the zopkio > > tests) before checking in this patch. I don't think we cleanly start and > > stop coordinator stream producers/consumers in all the managers. Please > > verify that nothing is broken due to this change. > > József Márton Jung wrote: > I have difficulties running the zopkio tests. The error message is the > following: > 2015-07-27 11:36:44,608 zopkio.remote_host_helper [ERROR] Error: > JAVA_HOME is not set and could not be found. > > JAVA_HOME is set on my machine (in /etc/profile, so it is available > system-wide), echoing it outputs the path to Java installation. I don't knw > what is wrong. > > Navina Ramesh wrote: > I think the problem is with the remote host (host to which zopkio is > trying to ssh) not having JAVA_HOME set correctly. Are you running the test > on a remote machine? > > József Márton Jung wrote: > I'm trying to run the tests locally, so zopkio is ssh-ing to localhost. > When I connect to localhost through ssh and when I try "echo $JAVA_HOME", it > prints the correct path to my Java home. I'm clueless at the moment. > > József Márton Jung wrote: > Okay, I figured out. It works. Yay! When deployment is going through SSH, > it is a non-interctive shell, therefore /etc/profile is not executed. > More details can be found here: > http://askubuntu.com/questions/247738/why-is-etc-profile-not-invoked-for-non-login-shells > When I added the following line before the line '# If not running > interactively, don't do anything' in ~/.bashrc: > ``` > export JAVA_HOME=/path/to/java/home > ``` > Integration tests started working. > > Maybe it would be a good idea to mention this on the page where > integration tests are described. > > Navina Ramesh wrote: > Duh.. that's true! It didn't occur to me. Glad you were able to figure it > out. > Yeah. We should probably mention this in the documentation.
I'll create a ticket for that. I ran the tests, the run was successful, the report tells that all tests passed. Logs are looking good, too. I'll align my branch with the current master, will run the tests again, and I'll attach the new patch. - József Márton ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36545/#review92938 ----------------------------------------------------------- On July 27, 2015, 10:15 a.m., József Márton Jung wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36545/ > ----------------------------------------------------------- > > (Updated July 27, 2015, 10:15 a.m.) > > > Review request for samza. > > > Repository: samza > > > Description > ------- > > The following has been refactored: > 1. Static inner classes from CoordinatorStreamMessage has been extracted > 2. Common functionality from CheckpointManager, ChangelogMappingManager and > LocalityManager has benn moved to a base class > > > Diffs > ----- > > checkstyle/import-control.xml 6654319 > samza-core/src/main/java/org/apache/samza/checkpoint/CheckpointManager.java > 7445996 > samza-core/src/main/java/org/apache/samza/container/LocalityManager.java > 55c258f > > samza-core/src/main/java/org/apache/samza/coordinator/stream/AbstractCoordinatorStreamManager.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamMessage.java > e5ab4fb > > samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemConsumer.java > b1078bd > > samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamSystemProducer.java > 92f8907 > > samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamWriter.java > f769756 > > samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/CoordinatorStreamMessage.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/Delete.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetChangelogMapping.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetCheckpoint.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetConfig.java > PRE-CREATION > > samza-core/src/main/java/org/apache/samza/coordinator/stream/messages/SetContainerHostMapping.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/job/model/JobModel.java ad6387d > > samza-core/src/main/java/org/apache/samza/storage/ChangelogPartitionManager.java > 7d3409c > samza-core/src/main/scala/org/apache/samza/coordinator/JobCoordinator.scala > f621611 > samza-core/src/main/scala/org/apache/samza/job/JobRunner.scala 1c178a6 > > samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamWrappedConsumer.java > e454593 > > samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamMessage.java > ac26a01 > > samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamSystemConsumer.java > c25f6a7 > > samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamSystemProducer.java > 1ef07d0 > > samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamWriter.java > c484660 > > samza-core/src/test/scala/org/apache/samza/container/TestSamzaContainer.scala > 84fdeaa > samza-yarn/src/main/resources/scalate/WEB-INF/views/index.scaml 41303f7 > > Diff: https://reviews.apache.org/r/36545/diff/ > > > Testing > ------- > > Tests has been updated. > > > Thanks, > > József Márton Jung > >
