You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by jhuynh1 <gi...@git.apache.org> on 2017/08/14 17:22:19 UTC
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
GitHub user jhuynh1 opened a pull request:
https://github.com/apache/geode/pull/712
GEODE-3434: Allow the modules to be interoperable with current and ol…
…der versions of tomcat 7
Modified DeltaSessions to use reflection to handle attributes fields incase an earlier tomcat 7 is used
Modified DeltaSession7 and DeltaSession8 to extend from DeltaSession
Added session backward compatibility tests
Modified aseembly build to download old product installs
Thank you for submitting a contribution to Apache Geode.
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?
- [ ] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [ ] Is your initial contribution a single, squashed commit?
- [ ] Does `gradlew build` run cleanly?
- [ ] 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)?
### 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. If you need help, please send an
email to dev@geode.apache.org.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/geode feature/GEODE-3434
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/geode/pull/712.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 #712
----
commit e9ed8a12f6f8cb9dfeedcec5922069bce2e7b7e3
Author: Jason Huynh <hu...@gmail.com>
Date: 2017-08-14T16:02:11Z
GEODE-3434: Allow the modules to be interoperable with current and older versions of tomcat 7
Modified DeltaSessions to use reflection to handle attributes fields incase an earlier tomcat 7 is used
Modified DeltaSession7 and DeltaSession8 to extend from DeltaSession
Added session backward compatibility tests
Modified aseembly build to download old product installs
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133827666
--- Diff: geode-old-versions/build.gradle ---
@@ -65,6 +92,21 @@ task createGeodeClasspathsFile {
new FileOutputStream(classpathsFile).withStream { fos ->
versions.store(fos, '')
}
+
+ installsFile.getParentFile().mkdirs();
+
+ new FileOutputStream(installsFile).withStream { fos ->
+ project.ext.installs.store(fos, '')
+ }
}
+
+ // Add sourceSets for backwards compatibility, rolling upgrade, and
+ // pdx testing.
+ addOldVersion('test100', '1.0.0-incubating')
+ addOldVersion('test110', '1.1.0')
+ addOldVersion('test111', '1.1.1')
--- End diff --
Maybe we should only download 1.2.0, if that's all we are testing against?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by jhuynh1 <gi...@git.apache.org>.
Github user jhuynh1 closed the pull request at:
https://github.com/apache/geode/pull/712
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by DivineEnder <gi...@git.apache.org>.
Github user DivineEnder commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133268501
--- Diff: geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventListener.java ---
@@ -34,7 +34,6 @@
import org.apache.geode.cache.query.internal.DefaultQuery;
import org.apache.geode.internal.cache.BucketNotFoundException;
import org.apache.geode.internal.cache.PrimaryBucketException;
-import org.apache.geode.internal.cache.tier.sockets.command.Default;
--- End diff --
Can you organize imports on this file since this import seems to have changed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133827419
--- Diff: extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java ---
@@ -586,6 +610,20 @@ public int getSizeInBytes() {
return serializedAttributes;
}
+ protected ConcurrentMap getAttributes() {
+ try {
+ Field field = getAttributesFieldObject();
+ field.setAccessible(true);
+ Map oldMap = (Map) field.get(this);
+ ConcurrentMap newMap = new ConcurrentHashMap();
--- End diff --
Why do we need to copy the map here? I think the attributes field is always set to a ConcurrentHashMap, even though in tomcat 6 the field was of type map.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by jhuynh1 <gi...@git.apache.org>.
Github user jhuynh1 commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133843663
--- Diff: geode-old-versions/build.gradle ---
@@ -65,6 +92,21 @@ task createGeodeClasspathsFile {
new FileOutputStream(classpathsFile).withStream { fos ->
versions.store(fos, '')
}
+
+ installsFile.getParentFile().mkdirs();
+
+ new FileOutputStream(installsFile).withStream { fos ->
+ project.ext.installs.store(fos, '')
+ }
}
+
+ // Add sourceSets for backwards compatibility, rolling upgrade, and
+ // pdx testing.
+ addOldVersion('test100', '1.0.0-incubating')
+ addOldVersion('test110', '1.1.0')
+ addOldVersion('test111', '1.1.1')
--- End diff --
Good idea, I'll add a boolean to decide whether to do the product install or not
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by jhuynh1 <gi...@git.apache.org>.
Github user jhuynh1 commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133843603
--- Diff: extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java ---
@@ -553,8 +555,30 @@ public void fromData(DataInput in) throws IOException, ClassNotFoundException {
}
}
- protected Map readInAttributes(final DataInput in) throws IOException, ClassNotFoundException {
- return DataSerializer.readObject(in);
+ private void readInAttributes(DataInput in) throws IOException, ClassNotFoundException {
+ Map map = DataSerializer.readObject(in);
+ ConcurrentMap newMap = new ConcurrentHashMap();
+ newMap.putAll(map);
+ try {
+ Field field = getAttributesFieldObject();
+ field.setAccessible(true);
+ field.set(this, newMap);
+ } catch (NoSuchFieldException e) {
+ logError(e);
--- End diff --
I'll throw NoSuchElementException and IllegalStateException if any of these occur
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by jhuynh1 <gi...@git.apache.org>.
Github user jhuynh1 commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133843558
--- Diff: extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java ---
@@ -586,6 +610,20 @@ public int getSizeInBytes() {
return serializedAttributes;
}
+ protected ConcurrentMap getAttributes() {
+ try {
+ Field field = getAttributesFieldObject();
+ field.setAccessible(true);
+ Map oldMap = (Map) field.get(this);
+ ConcurrentMap newMap = new ConcurrentHashMap();
--- End diff --
I think you are correct, I believe we wrapped it incase somehow it was not a concurrent hash map but I guess that was being overly cautious. I'll change them all to map
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by DivineEnder <gi...@git.apache.org>.
Github user DivineEnder commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133013329
--- Diff: geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventListener.java ---
@@ -34,6 +34,7 @@
import org.apache.geode.cache.query.internal.DefaultQuery;
import org.apache.geode.internal.cache.BucketNotFoundException;
import org.apache.geode.internal.cache.PrimaryBucketException;
+import org.apache.geode.internal.cache.tier.sockets.command.Default;
--- End diff --
Organize imports
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by DivineEnder <gi...@git.apache.org>.
Github user DivineEnder commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133012849
--- Diff: geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatSessionBackwardsCompatibilityTest.java ---
@@ -0,0 +1,254 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.session.tests;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.net.URISyntaxException;
+import java.util.Collection;
+import java.util.List;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestName;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import org.apache.geode.internal.AvailablePortHelper;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.management.internal.cli.util.CommandStringBuilder;
+import org.apache.geode.test.dunit.rules.GfshShellConnectionRule;
+import org.apache.geode.test.dunit.rules.LocatorServerStartupRule;
+import org.apache.geode.test.dunit.standalone.VersionManager;
+import org.apache.geode.test.junit.categories.BackwardCompatibilityTest;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory;
+
+/**
+ * This test iterates through the versions of Geode and executes session client compatibility with
+ * the current version of Geode.
+ */
+@Category({DistributedTest.class, BackwardCompatibilityTest.class})
+@RunWith(Parameterized.class)
+@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class)
+public class TomcatSessionBackwardsCompatibilityTest {
+
+ @Parameterized.Parameters
+ public static Collection<String> data() {
+ List<String> result = VersionManager.getInstance().getVersionsWithoutCurrent();
+ result.removeIf(s -> Integer.parseInt(s) < 120);
+ if (result.size() < 1) {
+ throw new RuntimeException("No older versions of Geode were found to test against");
+ }
+ return result;
+ }
+
+ @Rule
+ public transient GfshShellConnectionRule gfsh = new GfshShellConnectionRule();
+
+ @Rule
+ public transient LocatorServerStartupRule locatorStartup = new LocatorServerStartupRule();
+
+ @Rule
+ public transient TestName testName = new TestName();
+
+ public transient Client client;
+ public transient ContainerManager manager;
+
+ File oldBuild;
+ File oldModules;
+
+ TomcatInstall tomcat7079AndOldModules;
+ TomcatInstall tomcat7079AndCurrentModules;
+ TomcatInstall tomcat8AndOldModules;
+ TomcatInstall tomcat8AndCurrentModules;
+
+ int locatorPort;
+ String classPathTomcat7079;
+ String classPathTomcat8;
+
+ public TomcatSessionBackwardsCompatibilityTest(String version) {
+ VersionManager versionManager = VersionManager.getInstance();
+ String installLocation = versionManager.getInstall(version);
+ oldBuild = new File(installLocation);
+ oldModules = new File(installLocation + "/tools/Modules/");
+ }
+
+ protected void startServer(String name, String classPath, int locatorPort) throws Exception {
+ CommandStringBuilder command = new CommandStringBuilder(CliStrings.START_SERVER);
+ command.addOption(CliStrings.START_SERVER__NAME, name);
+ command.addOption(CliStrings.START_SERVER__SERVER_PORT, "0");
+ command.addOption(CliStrings.START_SERVER__CLASSPATH, classPath);
+ command.addOption(CliStrings.START_SERVER__LOCATORS, "localhost[" + locatorPort + "]");
+ gfsh.executeAndVerifyCommand(command.toString());
+ }
+
+ protected void startLocator(String name, String classPath, int port) throws Exception {
+ CommandStringBuilder locStarter = new CommandStringBuilder(CliStrings.START_LOCATOR);
+ locStarter.addOption(CliStrings.START_LOCATOR__MEMBER_NAME, "loc");
+ locStarter.addOption(CliStrings.START_LOCATOR__CLASSPATH, classPath);
+ locStarter.addOption(CliStrings.START_LOCATOR__PORT, Integer.toString(port));
+ gfsh.executeAndVerifyCommand(locStarter.toString());
+
+ }
+
+ @Before
+ public void setup() throws Exception {
+ tomcat7079AndOldModules = new TomcatInstall(TomcatInstall.TomcatVersion.TOMCAT779,
+ ContainerInstall.ConnectionType.CLIENT_SERVER,
+ ContainerInstall.DEFAULT_INSTALL_DIR + "Tomcat7079AndOldModules",
+ oldModules.getAbsolutePath(), oldBuild.getAbsolutePath() + "/lib");
+
+ tomcat7079AndCurrentModules = new TomcatInstall(TomcatInstall.TomcatVersion.TOMCAT779,
+ ContainerInstall.ConnectionType.CLIENT_SERVER,
+ ContainerInstall.DEFAULT_INSTALL_DIR + "Tomcat7079AndCurrentModules");
+
+ tomcat8AndOldModules = new TomcatInstall(TomcatInstall.TomcatVersion.TOMCAT8,
+ ContainerInstall.ConnectionType.CLIENT_SERVER,
+ ContainerInstall.DEFAULT_INSTALL_DIR + "Tomcat8AndOldModules", oldModules.getAbsolutePath(),
+ oldBuild.getAbsolutePath() + "/lib");
+
+ tomcat8AndCurrentModules = new TomcatInstall(TomcatInstall.TomcatVersion.TOMCAT8,
+ ContainerInstall.ConnectionType.CLIENT_SERVER,
+ ContainerInstall.DEFAULT_INSTALL_DIR + "Tomcat8AndCurrentModules");
+
+ classPathTomcat7079 = tomcat7079AndCurrentModules.getHome() + "/lib/*" + File.pathSeparator
+ + tomcat7079AndCurrentModules.getHome() + "/bin/*";
+ classPathTomcat8 = tomcat8AndCurrentModules.getHome() + "/lib/*" + File.pathSeparator
+ + tomcat8AndCurrentModules.getHome() + "/bin/*";
+
+ // Get available port for the locator
+ locatorPort = AvailablePortHelper.getRandomAvailableTCPPort();
+
+ tomcat7079AndOldModules.setDefaultLocator("localhost", locatorPort);
+ tomcat7079AndCurrentModules.setDefaultLocator("localhost", locatorPort);
+
+ tomcat8AndOldModules.setDefaultLocator("localhost", locatorPort);
+ tomcat8AndCurrentModules.setDefaultLocator("localhost", locatorPort);
+
+ client = new Client();
+ manager = new ContainerManager();
+ // Due to parameterization of the test name, the URI would be malformed. Instead, it strips off
+ // the [] symbols
+ manager.setTestName(testName.getMethodName().replace("[", "").replace("]", ""));
+ }
+
+ private void startClusterWithTomcat(String tomcatClassPath) throws Exception {
+ startLocator("loc", tomcatClassPath, locatorPort);
+ startServer("server", tomcatClassPath, locatorPort);
+ }
+
+ /**
+ * Stops all containers that were previously started and cleans up their configurations
+ */
+ @After
+ public void stop() throws Exception {
+ manager.stopAllActiveContainers();
+ manager.cleanUp();
+
+ tomcat8AndCurrentModules.clearPreviousRuns();
--- End diff --
Change clearPreviousRuns to remove only previous runs of current version being added
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by DivineEnder <gi...@git.apache.org>.
Github user DivineEnder commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133010884
--- Diff: geode-assembly/src/test/java/org/apache/geode/session/tests/ServerContainer.java ---
@@ -134,6 +134,7 @@ public ServerContainer(ContainerInstall install, File containerConfigHome,
// Set cacheXML file
File installXMLFile = install.getCacheXMLFile();
+ String path = logDir.getAbsolutePath() + "/" + installXMLFile.getName();
--- End diff --
Probably not needed
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by DivineEnder <gi...@git.apache.org>.
Github user DivineEnder commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133013984
--- Diff: geode-old-versions/build.gradle ---
@@ -65,6 +100,71 @@ task createGeodeClasspathsFile {
new FileOutputStream(classpathsFile).withStream { fos ->
versions.store(fos, '')
}
+
+ installsFile.getParentFile().mkdirs();
+
+ new FileOutputStream(installsFile).withStream { fos ->
+ project.ext.installs.store(fos, '')
+ }
}
+
+ // Add sourceSets for backwards compatibility, rolling upgrade, and
+// pdx testing.
+ addOldVersion('test100', '1.0.0-incubating')
+ addOldVersion('test110', '1.1.0')
+ addOldVersion('test111', '1.1.1')
+ addOldVersion('test120', '1.2.0')
+
+
+
+//
+// def downloadUrl = (geodeReleaseUrl != "") ? geodeReleaseUrl :
--- End diff --
Clean up commented code
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by upthewaterspout <gi...@git.apache.org>.
Github user upthewaterspout commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133827499
--- Diff: extensions/geode-modules/src/main/java/org/apache/geode/modules/session/catalina/DeltaSession.java ---
@@ -553,8 +555,30 @@ public void fromData(DataInput in) throws IOException, ClassNotFoundException {
}
}
- protected Map readInAttributes(final DataInput in) throws IOException, ClassNotFoundException {
- return DataSerializer.readObject(in);
+ private void readInAttributes(DataInput in) throws IOException, ClassNotFoundException {
+ Map map = DataSerializer.readObject(in);
+ ConcurrentMap newMap = new ConcurrentHashMap();
+ newMap.putAll(map);
+ try {
+ Field field = getAttributesFieldObject();
+ field.setAccessible(true);
+ field.set(this, newMap);
+ } catch (NoSuchFieldException e) {
+ logError(e);
--- End diff --
I think it would be better to throw an exception than log an error here, so it's obvious if something is broken here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by DivineEnder <gi...@git.apache.org>.
Github user DivineEnder commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133256792
--- Diff: geode-assembly/src/test/java/org/apache/geode/session/tests/ContainerInstall.java ---
@@ -18,6 +18,7 @@
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
+import java.net.URI;
--- End diff --
I think this is extraneous. It doesn't seem to be used anywhere in this file.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by DivineEnder <gi...@git.apache.org>.
Github user DivineEnder commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133011336
--- Diff: geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatInstall.java ---
@@ -131,15 +142,20 @@ public TomcatInstall(TomcatVersion version, ConnectionType connType) throws Exce
* within the installation's 'conf' folder, and {@link #updateProperties()} to set the jar
* skipping properties needed to speedup container startup.
*/
- public TomcatInstall(TomcatVersion version, ConnectionType connType, String installDir)
- throws Exception {
+ public TomcatInstall(TomcatVersion version, ConnectionType connType, String installDir,
+ String modulesJarLocation, String extraJarsPath) throws Exception {
// Does download and install from URL
- super(installDir, version.getDownloadURL(), connType, "tomcat");
+ super(installDir, version.getDownloadURL(), connType, "tomcat",
--- End diff --
Refactor both "tomcat" and DEFAULT_MODULE_LOCATION pass down/up into other constructors
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by jhuynh1 <gi...@git.apache.org>.
Github user jhuynh1 commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133270585
--- Diff: geode-lucene/src/main/java/org/apache/geode/cache/lucene/internal/LuceneEventListener.java ---
@@ -34,7 +34,6 @@
import org.apache.geode.cache.query.internal.DefaultQuery;
import org.apache.geode.internal.cache.BucketNotFoundException;
import org.apache.geode.internal.cache.PrimaryBucketException;
-import org.apache.geode.internal.cache.tier.sockets.command.Default;
--- End diff --
I reverted the first set of changes to this file as it didn't have anything to do with this change set. It shouldn't be showing up in this diff anymore but if it is I'll make sure to revert it...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] geode pull request #712: GEODE-3434: Allow the modules to be interoperable w...
Posted by DivineEnder <gi...@git.apache.org>.
Github user DivineEnder commented on a diff in the pull request:
https://github.com/apache/geode/pull/712#discussion_r133012057
--- Diff: geode-assembly/src/test/java/org/apache/geode/session/tests/TomcatInstall.java ---
@@ -150,6 +166,66 @@ public TomcatInstall(TomcatVersion version, ConnectionType connType, String inst
}
/**
+ * Copies jars specified by {@link #tomcatRequiredJars} from the {@link #getModulePath()} and the
+ * specified other directory passed to the function
+ *
+ * @throws IOException if the {@link #getModulePath()}, installation lib directory, or extra
+ * directory passed in contain no files.
+ */
+ private void copyTomcatGeodeReqFiles(String moduleJarDir, String extraJarsPath)
--- End diff --
Copy stuff in function into other function
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---