Skip to content
This repository was archived by the owner on Oct 14, 2020. It is now read-only.

Conversation

@mattbsox
Copy link
Member

Added parameters to the runtime adapter functions.
Pulled out common logic for the Liberty runtimes into liberty-common.
Moved the booster tests for Liberty into liberty-common.
Added a WLP runtime adapter.
Added a Liberty runtime adapter for boost-gradle.
Pulled out Docker and Spring logic from boost-gradle.

@mattbsox mattbsox force-pushed the runtimesWithParameters branch from 22f7863 to b1fd41a Compare August 21, 2019 16:03
@scottkurz
Copy link
Contributor

Seems to cover issue #341

@mattbsox
Copy link
Member Author

Need some reviews. Thanks! @chyt @scottkurz @cherylking @ajm01 @awisniew90 @uberskigeek

String runtime = System.getProperty('runtime')

if (libertyRuntime == null || libertyRuntime.isEmpty()) {
if (runtime == null || runtime.isEmpty()) {
Copy link
Member

Choose a reason for hiding this comment

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

The exception explicitly states they need to specify a Liberty runtime. Should it be more generic? Same for the runtimeVersion check below.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah yup, I'll change these to be boost specific.

} else {
runtimeGroup = "com.ibm.websphere.appserver.runtime"
runtimeArtifactId = "wlp-javaee7"
if (runtime == "ol") {
Copy link
Member

Choose a reason for hiding this comment

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

How do other runtimes get handled? Not understanding why we do something for "ol" specifically here.

project.dependencies.add('libertyRuntime', "io.openliberty:openliberty-runtime:$OPEN_LIBERTY_VERSION")
}
}
// project.liberty.server = configureBoostServerProperties()
Copy link
Member

Choose a reason for hiding this comment

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

Should this commented out code be deleted?

import boost.common.boosters.AbstractBoosterConfig;
import boost.common.config.BoosterConfigurator
import boost.common.config.BoostProperties;
import boost.common.utils.BoostUtil
Copy link
Member

Choose a reason for hiding this comment

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

The description on line 49 mentions Liberty. Should it? It depends on the specified runtime right?

<version>1.0-M1-SNAPSHOT</version> <scope>provided</scope> </dependency> -->
<dependency>
<groupId>boost.runtimes</groupId>
<artifactId>wlp</artifactId>
Copy link
Member

Choose a reason for hiding this comment

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

Is this artifactId correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Should I change that to liberty rather than wlp? Also need to update the version.

<dependency>
<groupId>net.wasdev.wlp.maven.plugins</groupId>
<artifactId>liberty-maven-plugin</artifactId>
<version>2.6.3</version>
Copy link
Member

Choose a reason for hiding this comment

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

Should update to new coordinates in openliberty.

private String libertyServerPath;

private MavenProject project;
private ExecutionEnvironment env;
Copy link
Member

Choose a reason for hiding this comment

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

Use new coordinates below for LMP.

public class WLPRuntime extends LibertyRuntime {

public WLPRuntime() {
super("com.ibm.websphere.appserver.runtime", "wlp-webProfile8", "19.0.0.7");
Copy link
Member

Choose a reason for hiding this comment

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

Seeing the runtime and version hardcoded seems odd. Is this a default that can be overridden somewhere by the end user?

compile "boost:boost-common:0.1.3-SNAPSHOT"
compile "boost.runtimes:liberty-common:0.1-SNAPSHOT"
compile "boost:boost-gradle-plugin:0.1.1-SNAPSHOT"
compile 'net.wasdev.wlp.gradle.plugins:liberty-gradle-plugin:2.6.6-SNAPSHOT'
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed we used 2.6.7-SNAPSHOT above.

public void doPackage() throws BoostException {
public void doPackage(List<AbstractBoosterConfig> boosterConfigs, Object mavenProject, Object pluginTask) throws BoostException {
project = (MavenProject)mavenProject;
env = ((AbstractMojo)pluginTask).getExecutionEnvironment();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think all the casting suggests that we should have two distinct interfaces one from boost-maven and one from boost-gradle, and let the runtime impls decide if and how they want to factor the common pieces. The interface isn't doing a good job at showing what needs to be implemented.

//String command = project.getProjectDir().toString() + "/gradlew";
try {
ProcessBuilder pb = new ProcessBuilder("gradle", taskName, "-i", "-s");
System.out.println("Executing task " + pb.command().get(1));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a pattern we've used anywhere before (kicking off another gradle invocation)? Or have seen anywhere? Just a bit hesitant to go too out on a limb.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants