Uploaded image for project: 'IGB'
  1. IGB
  2. IGBF-1366

Fix App Manager failure to load bundles when running in Development mode

    Details

    • Story Points:
      5.5
    • Sprint:
      Summer 2018 Part 2, Summer 2018 Part 3

      Description

      On the latest master, (commit af05261)
      The app manager does not show any apps.
      Even if remote and local repos are enabled.

        Attachments

          Issue Links

            Activity

            ieclabau Ivory Blakley (Inactive) created issue -
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment -

            This error appears on startup and on each time that I refresh an app repository:

            Exception in thread "JavaFX Application Thread" java.lang.NullPointerException
            at org.lorainelab.igb.plugin.manager.BundleInfoManager.lambda$isVersionOfBundleInstalled$48(BundleInfoManager.java:149)
            at java.util.stream.MatchOps$1MatchSink.accept(MatchOps.java:90)
            at java.util.Spliterators$ArraySpliterator.tryAdvance(Spliterators.java:958)
            at java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:126)
            at java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:498)
            at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:485)
            at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
            at java.util.stream.MatchOps$MatchOp.evaluateSequential(MatchOps.java:230)
            at java.util.stream.MatchOps$MatchOp.evaluateSequential(MatchOps.java:196)
            at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
            at java.util.stream.ReferencePipeline.anyMatch(ReferencePipeline.java:449)
            at org.lorainelab.igb.plugin.manager.BundleInfoManager.isVersionOfBundleInstalled(BundleInfoManager.java:149)
            at org.lorainelab.igb.plugin.manager.AppManagerFxPanel.lambda$null$5(AppManagerFxPanel.java:150)
            at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1374)
            at java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:580)
            at org.lorainelab.igb.plugin.manager.AppManagerFxPanel.lambda$udpateDataEventNotification$6(AppManagerFxPanel.java:149)
            at com.sun.javafx.application.PlatformImpl.lambda$null$173(PlatformImpl.java:295)
            at java.security.AccessController.doPrivileged(Native Method)
            at com.sun.javafx.application.PlatformImpl.lambda$runLater$174(PlatformImpl.java:294)
            at com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - This error appears on startup and on each time that I refresh an app repository: Exception in thread "JavaFX Application Thread" java.lang.NullPointerException at org.lorainelab.igb.plugin.manager.BundleInfoManager.lambda$isVersionOfBundleInstalled$48(BundleInfoManager.java:149) at java.util.stream.MatchOps$1MatchSink.accept(MatchOps.java:90) at java.util.Spliterators$ArraySpliterator.tryAdvance(Spliterators.java:958) at java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:126) at java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:498) at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:485) at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471) at java.util.stream.MatchOps$MatchOp.evaluateSequential(MatchOps.java:230) at java.util.stream.MatchOps$MatchOp.evaluateSequential(MatchOps.java:196) at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) at java.util.stream.ReferencePipeline.anyMatch(ReferencePipeline.java:449) at org.lorainelab.igb.plugin.manager.BundleInfoManager.isVersionOfBundleInstalled(BundleInfoManager.java:149) at org.lorainelab.igb.plugin.manager.AppManagerFxPanel.lambda$null$5(AppManagerFxPanel.java:150) at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1374) at java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:580) at org.lorainelab.igb.plugin.manager.AppManagerFxPanel.lambda$udpateDataEventNotification$6(AppManagerFxPanel.java:149) at com.sun.javafx.application.PlatformImpl.lambda$null$173(PlatformImpl.java:295) at java.security.AccessController.doPrivileged(Native Method) at com.sun.javafx.application.PlatformImpl.lambda$runLater$174(PlatformImpl.java:294) at com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment -

            When I checkout older commits and build everything is fine.
            But on the latest commit to master the app manager does not display any apps.

            The latest commit was to pull in the branch for IGBF-1098. I downloaded the installer for that branch, and IGB from that. In that, the app manager is fine. I can only assume there is some flaw in my process here, something that didn't get fully reset.

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - When I checkout older commits and build everything is fine. But on the latest commit to master the app manager does not display any apps. The latest commit was to pull in the branch for IGBF-1098 . I downloaded the installer for that branch, and IGB from that. In that, the app manager is fine. I can only assume there is some flaw in my process here, something that didn't get fully reset.
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment -

            When I used the installer from master, things are fine.

            Can anyone else reproduce this?
            Is this an issue with my machine? (mac)
            Could this be an issue with local builds or that doesn't affect the installer ?

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - When I used the installer from master, things are fine. Can anyone else reproduce this? Is this an issue with my machine? (mac) Could this be an issue with local builds or that doesn't affect the installer ?
            Hide
            kkorey Kiran Korey (Inactive) added a comment -

            I checked out master and tried to reproduce the issue in my system.
            But I was not able to do so.

            Show
            kkorey Kiran Korey (Inactive) added a comment - I checked out master and tried to reproduce the issue in my system. But I was not able to do so.
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment -

            After testing on multiple machines we learned this much more:

            Installers are good. Every time we run IGB using an installer, there is no error, all is well.
            The problem is introduced after running IGB through Netbeans.
            The problem is not limited to running IGB in Netbeans; after running IGB through Netbeans, running IGB through the command line generates the error.

            To demonstrate this...
            $ mvn clean install -DskipTests=true
            $ java -jar igb_exe.jar
            --> all is well
            (though some of the time the default repository is disabled, not sure why, but the apps appear once the repo is enabled)

            $ java -jar igb_exe.jar
            --> all is well
            Running it a second time demonstrates that this command can go through the entire program life-cycle without introducing the error.

            Now go to Netbeans. Run main.
            --> Error, no apps. (even with repos enabled)

            $ java -jar igb_exe.jar
            --> Error, no apps. (even with repos enabled)

            $ mvn clean install -DskipTests=true
            $ java -jar igb_exe.jar
            --> all is well

            Netbeans running IGB introduces the problem.
            The problem affects IGB even when run outside of Netbeans.
            Maven clean removes the problem.

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - After testing on multiple machines we learned this much more: Installers are good. Every time we run IGB using an installer, there is no error, all is well. The problem is introduced after running IGB through Netbeans. The problem is not limited to running IGB in Netbeans; after running IGB through Netbeans, running IGB through the command line generates the error. To demonstrate this... $ mvn clean install -DskipTests=true $ java -jar igb_exe.jar --> all is well (though some of the time the default repository is disabled, not sure why, but the apps appear once the repo is enabled) $ java -jar igb_exe.jar --> all is well Running it a second time demonstrates that this command can go through the entire program life-cycle without introducing the error. Now go to Netbeans. Run main. --> Error, no apps. (even with repos enabled) $ java -jar igb_exe.jar --> Error, no apps. (even with repos enabled) $ mvn clean install -DskipTests=true $ java -jar igb_exe.jar --> all is well Netbeans running IGB introduces the problem. The problem affects IGB even when run outside of Netbeans. Maven clean removes the problem.
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment - - edited

            Netbeans doesn't do magic. When it runs IGB its just using a system command just like we are when we run in the terminal.
            It just has a lot of details in its system command that we don't have.

            Here is the command Netbeans uses when you run main:
            (note, I have added newlines and spacing to make it easier to read)

            cd /Users/ivory/Documents/Repos/integrated-genome-browser/main;

            JAVA_HOME=/Library/Java/JavaVirtualMachines/jdk1.8.0_131.jdk/Contents/Home \
            "/Applications/NetBeans/NetBeans 8.2.app/Contents/Resources/NetBeans/java/maven/bin/mvn" \
            -Dexec.executable=/Library/Java/JavaVirtualMachines/jdk1.8.0_131.jdk/Contents/Home/bin/java \
            -Dexec.mainClass=com.affymetrix.main.Main \
            "-Dexec.args=-Xmx1g
            -DdevelopmentMode=true
            -Dapple.laf.useScreenMenuBar=true
            -Dsun.java2d.opengl=true
            -classpath %classpath com.affymetrix.main.Main" \
            org.codehaus.mojo:exec-maven-plugin:1.2.1:exec

            Copying that command from Netbeans and running it in the terminal is sufficient to produce the problem.

            (here is a copy/paste friendly version. same as above, but not spaces or newlines)
            cd /Users/ivory/Documents/Repos/integrated-genome-browser/main;
            JAVA_HOME=/Library/Java/JavaVirtualMachines/jdk1.8.0_131.jdk/Contents/Home "/Applications/NetBeans/NetBeans 8.2.app/Contents/Resources/NetBeans/java/maven/bin/mvn" -Dexec.executable=/Library/Java/JavaVirtualMachines/jdk1.8.0_131.jdk/Contents/Home/bin/java -Dexec.mainClass=com.affymetrix.main.Main "-Dexec.args=-Xmx1g -DdevelopmentMode=true -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - - edited Netbeans doesn't do magic. When it runs IGB its just using a system command just like we are when we run in the terminal. It just has a lot of details in its system command that we don't have. Here is the command Netbeans uses when you run main: (note, I have added newlines and spacing to make it easier to read) cd /Users/ivory/Documents/Repos/integrated-genome-browser/main; JAVA_HOME=/Library/Java/JavaVirtualMachines/jdk1.8.0_131.jdk/Contents/Home \ "/Applications/NetBeans/NetBeans 8.2.app/Contents/Resources/NetBeans/java/maven/bin/mvn" \ -Dexec.executable=/Library/Java/JavaVirtualMachines/jdk1.8.0_131.jdk/Contents/Home/bin/java \ -Dexec.mainClass=com.affymetrix.main.Main \ "-Dexec.args=-Xmx1g -DdevelopmentMode=true -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" \ org.codehaus.mojo:exec-maven-plugin:1.2.1:exec Copying that command from Netbeans and running it in the terminal is sufficient to produce the problem. (here is a copy/paste friendly version. same as above, but not spaces or newlines) cd /Users/ivory/Documents/Repos/integrated-genome-browser/main; JAVA_HOME=/Library/Java/JavaVirtualMachines/jdk1.8.0_131.jdk/Contents/Home "/Applications/NetBeans/NetBeans 8.2.app/Contents/Resources/NetBeans/java/maven/bin/mvn" -Dexec.executable=/Library/Java/JavaVirtualMachines/jdk1.8.0_131.jdk/Contents/Home/bin/java -Dexec.mainClass=com.affymetrix.main.Main "-Dexec.args=-Xmx1g -DdevelopmentMode=true -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment - - edited

            $ mvn clean install -DskipTests=true
            $ java -jar igb_exe.jar
            --> all is well

            $cd main
            $ mvn -Dexec.executable=/Library/Java/JavaVirtualMachines/jdk1.8.0_131.jdk/Contents/Home/bin/java -Dexec.mainClass=com.affymetrix.main.Main "-Dexec.args=-Xmx1g -DdevelopmentMode=true -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec
            --> error, no apps

            $ cd ..
            $ java -jar igb_exe.jar
            --> error, no apps

            So calling mvn with those args is sufficient to produce the problem (without Netbeans)
            ---------------------------------------------

            $ mvn clean install -DskipTests=true
            $ java -jar igb_exe.jar
            --> all is well

            $ git checkout 5aaf78603354239371f5080ebdfa5ffbe418932f

            $ mvn clean install -DskipTests=true
            $ java -jar igb_exe.jar
            --> all is well

            $ cd main
            $ mvn -Dexec.executable=/Library/Java/JavaVirtualMachines/jdk1.8.0_131.jdk/Contents/Home/bin/java -Dexec.mainClass=com.affymetrix.main.Main "-Dexec.args=-Xmx1g -DdevelopmentMode=true -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec
            --> all is well

            Therefore, on an older commit, that mvn command is NOT sufficient to produce the problem.

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - - edited $ mvn clean install -DskipTests=true $ java -jar igb_exe.jar --> all is well $cd main $ mvn -Dexec.executable=/Library/Java/JavaVirtualMachines/jdk1.8.0_131.jdk/Contents/Home/bin/java -Dexec.mainClass=com.affymetrix.main.Main "-Dexec.args=-Xmx1g -DdevelopmentMode=true -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec --> error, no apps $ cd .. $ java -jar igb_exe.jar --> error, no apps So calling mvn with those args is sufficient to produce the problem (without Netbeans) --------------------------------------------- $ mvn clean install -DskipTests=true $ java -jar igb_exe.jar --> all is well $ git checkout 5aaf78603354239371f5080ebdfa5ffbe418932f $ mvn clean install -DskipTests=true $ java -jar igb_exe.jar --> all is well $ cd main $ mvn -Dexec.executable=/Library/Java/JavaVirtualMachines/jdk1.8.0_131.jdk/Contents/Home/bin/java -Dexec.mainClass=com.affymetrix.main.Main "-Dexec.args=-Xmx1g -DdevelopmentMode=true -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec --> all is well Therefore, on an older commit, that mvn command is NOT sufficient to produce the problem.
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment -

            $ mvn clean install -DskipTests=true
            $ java -jar igb_exe.jar
            --> all is well
            $ mvn -Dexec.executable="java" "-Dexec.args=-Xmx1g -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec
            --> all is well (twice)
            $ mvn -Dexec.executable="java" "-Dexec.args=-Xmx1g -DdevelopmentMode=true -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec
            --> error, no apps

            So it looks like the -DdevelopmentMode=true argument has to be included to produce the error.
            Netbeans takes these arguments from nbactions.xml.
            That argument was present when the nbactions.xml file was added in 2014.

            I've had a hard time time finding anything to explain what these arguments do.
            It looks like this sets a system property which can be querried programatically useing System.getProperty("developmentMode")
            --assuming it is analogous to the 'CATALINA_OPTS' in this post: https://stackoverflow.com/questions/4907075/cross-server-environment-variable

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - $ mvn clean install -DskipTests=true $ java -jar igb_exe.jar --> all is well $ mvn -Dexec.executable="java" "-Dexec.args=-Xmx1g -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec --> all is well (twice) $ mvn -Dexec.executable="java" "-Dexec.args=-Xmx1g -DdevelopmentMode=true -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec --> error, no apps So it looks like the -DdevelopmentMode=true argument has to be included to produce the error. Netbeans takes these arguments from nbactions.xml. That argument was present when the nbactions.xml file was added in 2014. I've had a hard time time finding anything to explain what these arguments do. It looks like this sets a system property which can be querried programatically useing System.getProperty("developmentMode") --assuming it is analogous to the 'CATALINA_OPTS' in this post: https://stackoverflow.com/questions/4907075/cross-server-environment-variable
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment -

            What we know....

            The problem requires the most recent commit, so it must have something to do with the samtools library upgrade.
            The problem requires that the code with the upgrade is built.
            The problem requires that the program is run with -DdevelopmentMode=true.
            Whatever the problem is, it sticks around after the run is done. So subsequent runs are broken in the same way.
            The problem is removed by mvn clean install.

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - What we know.... The problem requires the most recent commit, so it must have something to do with the samtools library upgrade. The problem requires that the code with the upgrade is built. The problem requires that the program is run with -DdevelopmentMode=true. Whatever the problem is, it sticks around after the run is done. So subsequent runs are broken in the same way. The problem is removed by mvn clean install.
            ieclabau Ivory Blakley (Inactive) made changes -
            Field Original Value New Value
            Assignee Ann Loraine [ aloraine ] Ivory Blakley [ ieclabau ]
            ieclabau Ivory Blakley (Inactive) made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            ieclabau Ivory Blakley (Inactive) made changes -
            Link This issue blocks IGBF-1108 [ IGBF-1108 ]
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment - - edited

            Kiran found that apps/bundles are compared to all existing bundles, including all of IGB's internal modules.
            It compares their "symbolic name" to see if the bundle is already installed.
            One of those modules is returning null. The comparison brakes, with a null pointer exception.
            This comparison is done with each time that you add/refresh an app repo.
            Since the comparison fails, the whole process of displaying apps fails.

            The new code upgrades a library. I think its called htslib.
            Kiran found that the module that was returning null was placed between h and i in an alphabetical list.

            The process of adding modules may be different in development mode to allow developers to remove and add bundles more quickly.

            The new code builds a bundle that returns null for a name.

            ----------
            According to this site, the symbolic name is required. So our assumption that all bundles would have it is not all that bad.
            http://spring.io/blog/2008/02/18/creating-osgi-bundles/
            But I haven't found anything else to support that, so it may just be a Spring requirement, not an OSGi requirement, and thus not a solid assumption.

            I'm not sure what to expect to be different from Gradle bundles (like htslib).

            I looked to see what bundles are available in different modes of running IGB. For future reference, I saved a file (DiffLoadedBundles.xls) describing which bundles get listed in different modes.

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - - edited Kiran found that apps/bundles are compared to all existing bundles, including all of IGB's internal modules. It compares their "symbolic name" to see if the bundle is already installed. One of those modules is returning null. The comparison brakes, with a null pointer exception. This comparison is done with each time that you add/refresh an app repo. Since the comparison fails, the whole process of displaying apps fails. The new code upgrades a library. I think its called htslib. Kiran found that the module that was returning null was placed between h and i in an alphabetical list. The process of adding modules may be different in development mode to allow developers to remove and add bundles more quickly. The new code builds a bundle that returns null for a name. ---------- According to this site, the symbolic name is required. So our assumption that all bundles would have it is not all that bad. http://spring.io/blog/2008/02/18/creating-osgi-bundles/ But I haven't found anything else to support that, so it may just be a Spring requirement, not an OSGi requirement, and thus not a solid assumption. I'm not sure what to expect to be different from Gradle bundles (like htslib). I looked to see what bundles are available in different modes of running IGB. For future reference, I saved a file (DiffLoadedBundles.xls) describing which bundles get listed in different modes.
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment -

            I added a filter for null values.
            I also added a method to print a log statement when there are null values to alert developers. This is a separate method so that it does not have be processed in any of the many loops that are each called many times.

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - I added a filter for null values. I also added a method to print a log statement when there are null values to alert developers. This is a separate method so that it does not have be processed in any of the many loops that are each called many times.
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment -
            Show
            ieclabau Ivory Blakley (Inactive) added a comment - The fix is on this branch. https://bitbucket.org/IvoryBlak/integrated-genome-browser/branch/IGBF-1366_App_Manager_shows_no_apps I'm moving to first level review.
            ieclabau Ivory Blakley (Inactive) made changes -
            Status In Progress [ 3 ] Needs 1st Level Review [ 10005 ]
            ieclabau Ivory Blakley (Inactive) made changes -
            Assignee Ivory Blakley [ ieclabau ] Sneha Ramesh Watharkar [ jdaly ]
            ann.loraine Ann Loraine made changes -
            Assignee Sneha Ramesh Watharkar [ jdaly ] Anh Moss [ aduong ]
            aduong Anh Moss (Inactive) made changes -
            Status Needs 1st Level Review [ 10005 ] Reviewing [ 10301 ]
            Hide
            aduong Anh Moss (Inactive) added a comment -

            My testing verifies that this issue has been resolved. The App Manager now opens and displays all proper plugins. On a Mac OS, I ran IGB using the installer from main master, and the App Manager works properly. I also ran IGB with Netbeans, and the App Manager works properly. No error messages appear in the Netbeans IDE log upon startup, nor upon refreshing an app repository.

            I tried to test if local files/repos will load and run in the App Manager, but it did not work. Ivory stated that it may not be testable until this fix for the App Manager is merged in.

            Show
            aduong Anh Moss (Inactive) added a comment - My testing verifies that this issue has been resolved. The App Manager now opens and displays all proper plugins. On a Mac OS, I ran IGB using the installer from main master, and the App Manager works properly. I also ran IGB with Netbeans, and the App Manager works properly. No error messages appear in the Netbeans IDE log upon startup, nor upon refreshing an app repository. I tried to test if local files/repos will load and run in the App Manager, but it did not work. Ivory stated that it may not be testable until this fix for the App Manager is merged in.
            aduong Anh Moss (Inactive) made changes -
            Status Reviewing [ 10301 ] Ready for Pull Request [ 10304 ]
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment -

            I added comments in the code in response to Anns comments through bitbucket.

            In reading the description of the test, I worry that the test was not thorough enough to see the problem state and verify the fix. It just sounds like it verifies the fix.
            I think this should be reviewed by at least one of the other people who explored the problem in depth.

            I am re-assigning to Sneha to do a second test.

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - I added comments in the code in response to Anns comments through bitbucket. In reading the description of the test, I worry that the test was not thorough enough to see the problem state and verify the fix. It just sounds like it verifies the fix. I think this should be reviewed by at least one of the other people who explored the problem in depth. I am re-assigning to Sneha to do a second test.
            ieclabau Ivory Blakley (Inactive) made changes -
            Assignee Anh Moss [ aduong ] Sneha Ramesh Watharkar [ jdaly ]
            ieclabau Ivory Blakley (Inactive) made changes -
            Status Ready for Pull Request [ 10304 ] Needs 1st Level Review [ 10005 ]
            sneha Sneha Ramesh Watharkar (Inactive) made changes -
            Status Needs 1st Level Review [ 10005 ] Reviewing [ 10301 ]
            Hide
            sneha Sneha Ramesh Watharkar (Inactive) added a comment - - edited

            An error detected:
            After testing the application thrice, seems that uninstalling a plugin is not working when we run the application from netbeans. It goes to an unending loop with button saying "Uninstalling".
            I also checked it by running the application through command prompt and the issue did not come up. Installing and uninstalling both worked fine.
            However, The error which was appearing before about Null pointer exception when the bundle doesn't have a symbolic name in consoles of both command prompt and netbeans is not there anymore.

            Moving back to ToDo column and Reassigning to Ivory Blakley to investigate and look through the code of uninstalling process.

            Show
            sneha Sneha Ramesh Watharkar (Inactive) added a comment - - edited An error detected: After testing the application thrice, seems that uninstalling a plugin is not working when we run the application from netbeans. It goes to an unending loop with button saying "Uninstalling". I also checked it by running the application through command prompt and the issue did not come up. Installing and uninstalling both worked fine. However, The error which was appearing before about Null pointer exception when the bundle doesn't have a symbolic name in consoles of both command prompt and netbeans is not there anymore. Moving back to ToDo column and Reassigning to Ivory Blakley to investigate and look through the code of uninstalling process.
            sneha Sneha Ramesh Watharkar (Inactive) made changes -
            Status Reviewing [ 10301 ] Open [ 1 ]
            sneha Sneha Ramesh Watharkar (Inactive) made changes -
            Assignee Sneha Ramesh Watharkar [ jdaly ] Ivory Blakley [ ieclabau ]
            ann.loraine Ann Loraine made changes -
            Assignee Ivory Blakley [ ieclabau ] Sneha Ramesh Watharkar [ jdaly ]
            sneha Sneha Ramesh Watharkar (Inactive) made changes -
            Assignee Sneha Ramesh Watharkar [ jdaly ] Ivory Blakley [ ieclabau ]
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment -

            I think the original issue had multiple symptoms. The core issue is that there is a bundle that has symbolic name: null. One symptom is that the app manager was not able to display apps. Another symptom is the inability to uninstall apps, but that symptom is un-detectable when you can't display apps in the first place.

            I was not able to fix the core issue.
            The core fix would be to add some step that gives a symbolic name to any bundle that didn't have one.
            The installed bundled are of class BundleImpl
            org/apache/felix/framework/BundleImpl.java
            I don't see any way to change the symbolic name attribute.

            Another core fix might be to change this bundle, maybe wrap it up as a new bundle that does have symbolic name even in developer mode. But as a rule we want to avoid modifying the bundles; better to use them as they are, so we can easily swap it out for an updated version.

            Another core fix might just be for the htsjdk to be updated into a bundle that Does have a symbolic name (we should reach out to the developers to ask about this).

            My current fix is not a core fix. It addressed the symptom by side-stepping the null pointer exception. Thus, fixing one symptom reveals another.

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - I think the original issue had multiple symptoms. The core issue is that there is a bundle that has symbolic name: null. One symptom is that the app manager was not able to display apps. Another symptom is the inability to uninstall apps, but that symptom is un-detectable when you can't display apps in the first place. I was not able to fix the core issue. The core fix would be to add some step that gives a symbolic name to any bundle that didn't have one. The installed bundled are of class BundleImpl org/apache/felix/framework/BundleImpl.java I don't see any way to change the symbolic name attribute. Another core fix might be to change this bundle, maybe wrap it up as a new bundle that does have symbolic name even in developer mode. But as a rule we want to avoid modifying the bundles; better to use them as they are, so we can easily swap it out for an updated version. Another core fix might just be for the htsjdk to be updated into a bundle that Does have a symbolic name (we should reach out to the developers to ask about this). My current fix is not a core fix. It addressed the symptom by side-stepping the null pointer exception. Thus, fixing one symptom reveals another.
            ann.loraine Ann Loraine made changes -
            Comment [ Please clarify:
            "However, The error in consoles of both command prompt and netbeans is not there anymore."
            ]
            ann.loraine Ann Loraine made changes -
            Comment [ Please edit previous comment to clarify what this means:

            "However, The error in consoles of both command prompt and netbeans is not there anymore." ]
            ann.loraine Ann Loraine made changes -
            Comment [ Done. ]
            Hide
            ann.loraine Ann Loraine added a comment -

            Please identify which commit introduces the problem.

            Show
            ann.loraine Ann Loraine added a comment - Please identify which commit introduces the problem.
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment - - edited

            The following test confirms that this is another symptom of the original core issue.

            1. installer
              I used the installer from my branch (IGB-IGBF-1366_App_Manager_shows_no_apps.dmg).
              -->all is well -->IGB runs without any errors, and it can display, install and uninstall Apps.

            $ mvn clean install -DskipTests=true

            1. run simplest way
              $ java -jar igb_run.jar
              -->all is well
            2. run using mvn NOT in developer mode
              $ mvn -Dexec.executable="java" "-Dexec.args=-Xmx1g -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec
              -->all is well
            3. run using mvn in developer mode
              $ mvn -Dexec.executable="java" "-Dexec.args=-Xmx1g -DdevelopmentMode=true -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec
              -->Error: Apps displya, and install, BUT cannot uninstall.
            4. run using mvn NOT in developer mode (identical to before, but now the state has changed because we have run it in dev mode)
              $ mvn -Dexec.executable="java" "-Dexec.args=-Xmx1g -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec
              -->Error: Apps display, and install, BUT cannot uninstall.

            So even though this bug can only be seen on my branch, it was not caused by any of my changes. There is no error message so it may take slightly longer to track down the method that is throwing the error now.

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - - edited The following test confirms that this is another symptom of the original core issue. installer I used the installer from my branch (IGB- IGBF-1366 _App_Manager_shows_no_apps.dmg). -->all is well -->IGB runs without any errors, and it can display, install and uninstall Apps. $ mvn clean install -DskipTests=true run simplest way $ java -jar igb_run.jar -->all is well run using mvn NOT in developer mode $ mvn -Dexec.executable="java" "-Dexec.args=-Xmx1g -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec -->all is well run using mvn in developer mode $ mvn -Dexec.executable="java" "-Dexec.args=-Xmx1g -DdevelopmentMode=true -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec -->Error: Apps displya, and install, BUT cannot uninstall. run using mvn NOT in developer mode (identical to before, but now the state has changed because we have run it in dev mode) $ mvn -Dexec.executable="java" "-Dexec.args=-Xmx1g -Dapple.laf.useScreenMenuBar=true -Dsun.java2d.opengl=true -classpath %classpath com.affymetrix.main.Main" org.codehaus.mojo:exec-maven-plugin:1.2.1:exec -->Error: Apps display, and install, BUT cannot uninstall. So even though this bug can only be seen on my branch, it was not caused by any of my changes. There is no error message so it may take slightly longer to track down the method that is throwing the error now.
            ieclabau Ivory Blakley (Inactive) made changes -
            Attachment DiffLoadedBundles.xls [ 14134 ]
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment - - edited

            Kiran and I reviewed the meta data files for several packages.
            We need this file:
            com/github/samtools/htsjdk/2.15.1/htsjdk-2.15.1.jar:META-INF/MANIFEST.MF
            to have this line:
            Bundle-SymbolicName: <symbolic name>
            Bundle-Version: <version>

            To test this, we cloned the samtools github repository, and modified the jar section in the file build.gradle.
            Original:
            jar {
            manifest

            { attributes 'Implementation-Title': 'HTSJDK', 'Implementation-Vendor' : 'Samtools Organization', 'Implementation-Version': version }

            }

            With our modification:
            jar {
            manifest

            { attributes 'Implementation-Title': 'HTSJDK', 'Implementation-Vendor' : 'Samtools Organization', 'Implementation-Version': version, 'Bundle-SymbolicName': project.getGroup()+'.'+project.getName(), 'Bundle-Version': version }

            }

            Kiran built a jar, modified the jar name to match the existing jar file, and used that jar file in place of the current htsjdk jar.
            IGB main master was able to build and run without errors.

            We need to either:

            • host this jar file ourselves, and make this same modification every time we update the library.
              OR
            • maybe we can submit a pull request to com.github.samtools and they can add these two lines to their file so that version in their repo works in our system.
            Show
            ieclabau Ivory Blakley (Inactive) added a comment - - edited Kiran and I reviewed the meta data files for several packages. We need this file: com/github/samtools/htsjdk/2.15.1/htsjdk-2.15.1.jar:META-INF/MANIFEST.MF to have this line: Bundle-SymbolicName: <symbolic name> Bundle-Version: <version> To test this, we cloned the samtools github repository, and modified the jar section in the file build.gradle. Original: jar { manifest { attributes 'Implementation-Title': 'HTSJDK', 'Implementation-Vendor' : 'Samtools Organization', 'Implementation-Version': version } } With our modification: jar { manifest { attributes 'Implementation-Title': 'HTSJDK', 'Implementation-Vendor' : 'Samtools Organization', 'Implementation-Version': version, 'Bundle-SymbolicName': project.getGroup()+'.'+project.getName(), 'Bundle-Version': version } } Kiran built a jar, modified the jar name to match the existing jar file, and used that jar file in place of the current htsjdk jar. IGB main master was able to build and run without errors. We need to either: host this jar file ourselves, and make this same modification every time we update the library. OR maybe we can submit a pull request to com.github.samtools and they can add these two lines to their file so that version in their repo works in our system.
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment - - edited

            The version cheat:
            In order to test the above, we had to cheat with the version number to make it match our current pom files.
            The file has a line that defines the version:

            • version = isRelease ? gitVersion : gitVersion + "-SNAPSHOT"

            Since we can't run it in release mode (partly because I don't know how and partly because that activates their automated update process), we just hard-coded the version to match our current pom files:

            • version = '2.15.1'

            A real solution will have to address that a bit more elegantly.
            ____________________

            The easy plan:
            Kiran reached out to the samtools developers asking them to include the lines we need in their build. If they can add the lines we need to their build, then we don't have to do any customization on our end.

            The DIY plan:
            This is what we'll have to do if we don't hear back from the samtools developers, or if they say they don't want to add those lines, or it might take a long time.

            We'll host the modified jar file ourselves.
            To do this, we need to :

            • make a fork of the htsjdk repo
            • add a commit that adds the lines we need and adds this line for the version
              version = gitVersion
              'Bundle-SymbolicName': project.getGroup() + '.' + project.getName() + '-igb'
              'Bundle-Version': version
            • build the jar file and add that to our repo
            • modify the IGB pom files to use that version number

            When we want to update the library, we would need to:

            • rebase our fork based on the main htsjdk repo, applying our one commit on top.
            • build the jar file and put it in our repo
            • update the version number in our pom files.

            Using +'-igb' ensures that IGB will pull our version of the jar file even if it has access to a repository that has the non-modified htsjdk version.
            ____________________

            Kiran reached out to the samtools developers.
            They do pretty frequent releases and they have been responsive before, so I have high hopes for the 'easy plan'.
            If we haven't heard back from them by Tuesday (31 July 2018) then we'll move forward with the DIY plan.

            In the mean time, we'll use our test jar file or installers to do the testing for issues that are affected by this.

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - - edited The version cheat: In order to test the above, we had to cheat with the version number to make it match our current pom files. The file has a line that defines the version: version = isRelease ? gitVersion : gitVersion + "-SNAPSHOT" Since we can't run it in release mode (partly because I don't know how and partly because that activates their automated update process), we just hard-coded the version to match our current pom files: version = '2.15.1' A real solution will have to address that a bit more elegantly. ____________________ The easy plan: Kiran reached out to the samtools developers asking them to include the lines we need in their build. If they can add the lines we need to their build, then we don't have to do any customization on our end. The DIY plan: This is what we'll have to do if we don't hear back from the samtools developers, or if they say they don't want to add those lines, or it might take a long time. We'll host the modified jar file ourselves. To do this, we need to : make a fork of the htsjdk repo add a commit that adds the lines we need and adds this line for the version version = gitVersion 'Bundle-SymbolicName': project.getGroup() + '.' + project.getName() + '-igb' 'Bundle-Version': version build the jar file and add that to our repo modify the IGB pom files to use that version number When we want to update the library, we would need to: rebase our fork based on the main htsjdk repo, applying our one commit on top. build the jar file and put it in our repo update the version number in our pom files. Using +'-igb' ensures that IGB will pull our version of the jar file even if it has access to a repository that has the non-modified htsjdk version. ____________________ Kiran reached out to the samtools developers. They do pretty frequent releases and they have been responsive before, so I have high hopes for the 'easy plan'. If we haven't heard back from them by Tuesday (31 July 2018) then we'll move forward with the DIY plan. In the mean time, we'll use our test jar file or installers to do the testing for issues that are affected by this.
            Hide
            ann.loraine Ann Loraine added a comment -

            Don't use their snapshot release. Use their most recent stable release in our project.

            Show
            ann.loraine Ann Loraine added a comment - Don't use their snapshot release. Use their most recent stable release in our project.
            ieclabau Ivory Blakley (Inactive) made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment -

            Kiran forked the htsjdk repo and made a commit with the changes we need and made a pull request.
            See https://github.com/samtools/htsjdk/pull/1169
            _____________________

            I forked the htsjdk repo and made a branch called IGB_compatible.
            https://github.com/IvoryC/htsjdk/tree/IGB_compatible
            I pulled the commit that Kiran made.
            I made a commit to add '-igb' to the project name and set the version to use the format it would use for a release.
            I rebased this branch (with its 2 commits) back to the last stable release, which was commit 96ce8fb for version 2.16.0.
            To check for new releases to repeat this, see: https://github.com/samtools/htsjdk/releases

            On the IGB_compatible branch, I ran this command to build a jar file:
            ./gradlew jar
            This produced a jar file called htsjdk-igb-2.16.0.jar

            To test this, I copied this jar file to my .m2 directory and renamed it to match the version that the IGB pom files are expecting.
            I can open the manefest file in netbeans and see that it has a symbolic name attribute.
            On the IGB master branch, I ran $ mvn clean install and ran igb with dev mode = true.
            -->all is well, Apps display, install and uninstall.
            _______________________

            Nest steps:

            • We need to change the IGB pom files to take this jar file.
            • We need to put this jar file in the repository so anyone who builds IGB can access it.
            Show
            ieclabau Ivory Blakley (Inactive) added a comment - Kiran forked the htsjdk repo and made a commit with the changes we need and made a pull request. See https://github.com/samtools/htsjdk/pull/1169 _____________________ I forked the htsjdk repo and made a branch called IGB_compatible. https://github.com/IvoryC/htsjdk/tree/IGB_compatible I pulled the commit that Kiran made. I made a commit to add '-igb' to the project name and set the version to use the format it would use for a release. I rebased this branch (with its 2 commits) back to the last stable release, which was commit 96ce8fb for version 2.16.0. To check for new releases to repeat this, see: https://github.com/samtools/htsjdk/releases On the IGB_compatible branch, I ran this command to build a jar file: ./gradlew jar This produced a jar file called htsjdk-igb-2.16.0.jar To test this, I copied this jar file to my .m2 directory and renamed it to match the version that the IGB pom files are expecting. I can open the manefest file in netbeans and see that it has a symbolic name attribute. On the IGB master branch, I ran $ mvn clean install and ran igb with dev mode = true. -->all is well, Apps display, install and uninstall. _______________________ Nest steps: We need to change the IGB pom files to take this jar file. We need to put this jar file in the repository so anyone who builds IGB can access it.
            Hide
            ann.loraine Ann Loraine added a comment -

            For this to be used as a bundle, it would need to include export statements for publicly exposed packages.
            How is it working without these additional meta-data in the manifest file?

            Show
            ann.loraine Ann Loraine added a comment - For this to be used as a bundle, it would need to include export statements for publicly exposed packages. How is it working without these additional meta-data in the manifest file?
            ann.loraine Ann Loraine made changes -
            Fix Version/s 9.0.2 Minor Release [ 10600 ]
            ann.loraine Ann Loraine made changes -
            Link This issue relates to IGBF-1371 [ IGBF-1371 ]
            ann.loraine Ann Loraine made changes -
            Assignee Ivory Blakley [ ieclabau ] Kiran Korey [ kkorey ]
            ann.loraine Ann Loraine made changes -
            Summary App manager is broken Fix App Manager failure to load bundles when running in Development mode
            Hide
            ann.loraine Ann Loraine added a comment -

            Requesting Kiran create Powerpoint case study describing problem and fix

            Show
            ann.loraine Ann Loraine added a comment - Requesting Kiran create Powerpoint case study describing problem and fix
            ann.loraine Ann Loraine made changes -
            Story Points 5.5
            ann.loraine Ann Loraine made changes -
            Rank Ranked higher
            Hide
            kkorey Kiran Korey (Inactive) added a comment -

            The fix for the issue is in:

            https://bitbucket.org/kkorey/kkorey-igb/branch/IGBF-1098-sam-so

            It also has the code to IGBF-1371.

            Show
            kkorey Kiran Korey (Inactive) added a comment - The fix for the issue is in: https://bitbucket.org/kkorey/kkorey-igb/branch/IGBF-1098-sam-so It also has the code to IGBF-1371 .
            ann.loraine Ann Loraine made changes -
            Sprint Summer 2018 Part 2 [ 49 ] Summer 2018 Part 2, Summer 2018 Part 3 [ 49, 50 ]
            ann.loraine Ann Loraine made changes -
            Rank Ranked higher
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment - - edited

            Comments on branch https://bitbucket.org/kkorey/kkorey-igb/branch/IGBF-1098-sam-so:

            This is one commit that does several very separate things.
            Most of the changes are what the commit message says "Removed references to older net.sf.samtools/sam-ext library." (which addresses issue IGBF-1371)
            But the changes in some files are not related to that at all, and warrant their own commit with their own message.

            Most notably, one change in core/shared-lib-wrapper/pom.xml alters how the htsjdk bundle is handled and addresses the problem discussed in IGBF-1366.
            Although that change is very small, I think it warrants its own commit message.

            Other potentially un-related changes include:

            • In file core/quickload/pom.xml, added the scope of "provided" to the dependency org.lorainelab.igb (why was this change needed?)
            • In the file optionalPlugins/geometric-mean/pom.xml, it looks like a line was previously just in the wrong place . Moving that line doesn't look like it has anything to do with removing the net.sf.samtools library. It is conceivable that its not just in the wrong place, it was there for a reason, and tracking down this 'fix' later when we find problems will be easier if the change is explained with its own commit. The commit message could be as simple as just 'formatting correction in pom file', and that is more informative than saying the change was made as part of removing the net.sf.samtools library. If this change has some functional motivation, then please explain that.

            I think the easiest way to separate these changes into separate commits will be to make a new branch and re-produce these changes.
            I am stopping my review now to allow for these updates.

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - - edited Comments on branch https://bitbucket.org/kkorey/kkorey-igb/branch/IGBF-1098-sam-so: This is one commit that does several very separate things. Most of the changes are what the commit message says "Removed references to older net.sf.samtools/sam-ext library." (which addresses issue IGBF-1371 ) But the changes in some files are not related to that at all, and warrant their own commit with their own message. Most notably, one change in core/shared-lib-wrapper/pom.xml alters how the htsjdk bundle is handled and addresses the problem discussed in IGBF-1366 . Although that change is very small, I think it warrants its own commit message. Other potentially un-related changes include: In file core/quickload/pom.xml, added the scope of "provided" to the dependency org.lorainelab.igb (why was this change needed?) In the file optionalPlugins/geometric-mean/pom.xml, it looks like a line was previously just in the wrong place . Moving that line doesn't look like it has anything to do with removing the net.sf.samtools library. It is conceivable that its not just in the wrong place, it was there for a reason, and tracking down this 'fix' later when we find problems will be easier if the change is explained with its own commit. The commit message could be as simple as just 'formatting correction in pom file', and that is more informative than saying the change was made as part of removing the net.sf.samtools library. If this change has some functional motivation, then please explain that. I think the easiest way to separate these changes into separate commits will be to make a new branch and re-produce these changes. I am stopping my review now to allow for these updates.
            Hide
            kkorey Kiran Korey (Inactive) added a comment -
            Show
            kkorey Kiran Korey (Inactive) added a comment - Ivory Blakley Can you please review the changes in this branch https://bitbucket.org/kkorey/kkorey-igb/branch/IGBF-1371-library
            kkorey Kiran Korey (Inactive) made changes -
            Status In Progress [ 3 ] Needs 1st Level Review [ 10005 ]
            ieclabau Ivory Blakley (Inactive) made changes -
            Assignee Kiran Korey [ kkorey ] Ivory Blakley [ ieclabau ]
            ieclabau Ivory Blakley (Inactive) made changes -
            Status Needs 1st Level Review [ 10005 ] Reviewing [ 10301 ]
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment - - edited

            Much clearer!

            It looks like the changes for the 1366 issue were removed, so I assume that will be another branch ...?
            __________________

            The commit for removing the net.sf.samtools is clearly just that.
            And the formatting fixes are very simple and clearly just that.
            So the code-review and the git review are good. : )

            These changes are minor and only affect the pom file. We have already removed all of the import statements for the samtools library.
            To make sure IGB is able to build without the net.sf.sam-exec library, I removed it from my .m2, and I cleared IGB's bundle directory, and I ran $mvn clean install.
            IGB was able to build with tests, without errors, and run. The net.sf.sam-exec was not re-downloaded to .m2, so it looks like IGB is completely free of the dependency.
            That's the only functional review I can think of for this, and it looks good. : )

            __________________

            I'm copying these comments to issue IGBF-1371.
            Branch IGBF-1371-library has to do with that issue, and now has nothing to do with this issue (IGBF-1366).
            I'm not going to make any changes to this issue (except to assign back to Kiran since he's doing the ppt, and making a branch for this issue).

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - - edited Much clearer! It looks like the changes for the 1366 issue were removed, so I assume that will be another branch ...? __________________ The commit for removing the net.sf.samtools is clearly just that. And the formatting fixes are very simple and clearly just that. So the code-review and the git review are good. : ) These changes are minor and only affect the pom file. We have already removed all of the import statements for the samtools library. To make sure IGB is able to build without the net.sf.sam-exec library, I removed it from my .m2, and I cleared IGB's bundle directory, and I ran $mvn clean install. IGB was able to build with tests, without errors, and run. The net.sf.sam-exec was not re-downloaded to .m2, so it looks like IGB is completely free of the dependency. That's the only functional review I can think of for this, and it looks good. : ) __________________ I'm copying these comments to issue IGBF-1371 . Branch IGBF-1371 -library has to do with that issue, and now has nothing to do with this issue ( IGBF-1366 ). I'm not going to make any changes to this issue (except to assign back to Kiran since he's doing the ppt, and making a branch for this issue).
            ieclabau Ivory Blakley (Inactive) made changes -
            Status Reviewing [ 10301 ] In Progress [ 3 ]
            ieclabau Ivory Blakley (Inactive) made changes -
            Assignee Ivory Blakley [ ieclabau ] Kiran Korey [ kkorey ]
            Hide
            kkorey Kiran Korey (Inactive) added a comment -

            I have merged in the changes for this issue as well in the same branch as 1371 as it was just removing an artifact.
            Please do take a look at it once.

            Show
            kkorey Kiran Korey (Inactive) added a comment - I have merged in the changes for this issue as well in the same branch as 1371 as it was just removing an artifact. Please do take a look at it once.
            ieclabau Ivory Blakley (Inactive) made changes -
            Status In Progress [ 3 ] Reviewing [ 10301 ]
            ieclabau Ivory Blakley (Inactive) made changes -
            Assignee Kiran Korey [ kkorey ] Ivory Blakley [ ieclabau ]
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment - - edited

            Reviewing the added commit:

            git review: the message matches the change. The issue reference is for issue IGBF-1371 but IGBF-1366 would be a better explanation of why that change was made.

            code review: short and sweet. good.

            functional review: IGB builds and runs without error. In the app manager, Apps display, install, and uninstall.

            This change is much better than working around the null pointer, and much better than modifying the jar file itself.

            I am reassigning to Kiran to modify the commit message, and submit a pull request.
            The issue will be moved back to in-progress while Kiran finishes the ppt associated with this issue. But IGBF-1371 uses the same branch and it is is "ready for pull request".

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - - edited Reviewing the added commit: git review: the message matches the change. The issue reference is for issue IGBF-1371 but IGBF-1366 would be a better explanation of why that change was made. code review: short and sweet. good. functional review: IGB builds and runs without error. In the app manager, Apps display, install, and uninstall. This change is much better than working around the null pointer, and much better than modifying the jar file itself. I am reassigning to Kiran to modify the commit message, and submit a pull request. The issue will be moved back to in-progress while Kiran finishes the ppt associated with this issue. But IGBF-1371 uses the same branch and it is is "ready for pull request".
            ieclabau Ivory Blakley (Inactive) made changes -
            Status Reviewing [ 10301 ] Ready for Pull Request [ 10304 ]
            ieclabau Ivory Blakley (Inactive) made changes -
            Status Ready for Pull Request [ 10304 ] In Progress [ 3 ]
            ieclabau Ivory Blakley (Inactive) made changes -
            Assignee Ivory Blakley [ ieclabau ] Kiran Korey [ kkorey ]
            kkorey Kiran Korey (Inactive) made changes -
            Status In Progress [ 3 ] Needs 1st Level Review [ 10005 ]
            ann.loraine Ann Loraine made changes -
            Status Needs 1st Level Review [ 10005 ] Reviewing [ 10301 ]
            ann.loraine Ann Loraine made changes -
            Status Reviewing [ 10301 ] Ready for Pull Request [ 10304 ]
            Hide
            ann.loraine Ann Loraine added a comment -

            Please rebase on latest master & submit pull request.

            Show
            ann.loraine Ann Loraine added a comment - Please rebase on latest master & submit pull request.
            Hide
            kkorey Kiran Korey (Inactive) added a comment -

            This issue does not have any code changes. The code change is done as part of IGBF-1371 (Which is already mearged and moved to needs testing)
            This is pending only because of the Case study document.

            Show
            kkorey Kiran Korey (Inactive) added a comment - This issue does not have any code changes. The code change is done as part of IGBF-1371 (Which is already mearged and moved to needs testing) This is pending only because of the Case study document.
            ann.loraine Ann Loraine made changes -
            Resolution Done [ 10000 ]
            Status Ready for Pull Request [ 10304 ] Closed [ 6 ]
            ann.loraine Ann Loraine made changes -
            Labels Advanced
            ann.loraine Ann Loraine made changes -
            Workflow Loraine Lab Workflow [ 18062 ] Fall 2019 Workflow Update [ 19927 ]
            ann.loraine Ann Loraine made changes -
            Workflow Fall 2019 Workflow Update [ 19927 ] Revised Fall 2019 Workflow Update [ 22047 ]

              People

              • Assignee:
                kkorey Kiran Korey (Inactive)
                Reporter:
                ieclabau Ivory Blakley (Inactive)
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: