Skip to content

Commit

Permalink
Partially Address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
shardulm94 committed Apr 6, 2019
1 parent 50a05ab commit a91bee5
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,12 @@ dependencies {
compile('org.apache.commons:commons-io:1.3.2')
}

licenseHive.enabled = false
licensePresto.enabled = false
licenseSpark.enabled = false
// If the license plugin is applied, disable license checks for the autogenerated source sets
plugins.withId('com.github.hierynomus.license') {
licenseHive.enabled = false
licensePresto.enabled = false
licenseSpark.enabled = false
}

// TODO: Add a debugPlatform flag to allow debugging specific test methods in IntelliJ
// for a particular platform other than default
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@
import com.linkedin.transport.codegen.PrestoWrapperGenerator;
import com.linkedin.transport.codegen.SparkWrapperGenerator;
import java.io.IOException;
import java.io.InputStream;
import java.util.List;
import java.util.Properties;

import static com.linkedin.transport.plugin.DependencyConfigurationName.*;
import static com.linkedin.transport.plugin.DependencyConfigurationType.*;


/**
Expand All @@ -25,13 +26,14 @@ private Defaults() {
}

// The versions of the Transport and supported platforms to apply corresponding versions of the platform dependencies
private static final Properties DEFAULT_VERSIONS;
private static final Properties DEFAULT_VERSIONS = loadDefaultVersions();

static {
DEFAULT_VERSIONS = new Properties();
try {
DEFAULT_VERSIONS.load(
Thread.currentThread().getContextClassLoader().getResourceAsStream("version-info.properties"));
private static Properties loadDefaultVersions () {
try(InputStream is =
Thread.currentThread().getContextClassLoader().getResourceAsStream("version-info.properties")) {
Properties defaultVersions = new Properties();
defaultVersions.load(is);
return defaultVersions;
} catch (IOException e) {
throw new RuntimeException("Error loading version-info.properties", e);
}
Expand All @@ -48,8 +50,8 @@ private Defaults() {
getDependencyConfiguration(RUNTIME_ONLY, "com.linkedin.transport:transportable-udfs-test-generic", "transport")
);

static final List<PlatformConfiguration> DEFAULT_PLATFORMS = ImmutableList.of(
new PlatformConfiguration(
static final List<Platform> DEFAULT_PLATFORMS = ImmutableList.of(
new Platform(
"presto",
Language.JAVA,
PrestoWrapperGenerator.class,
Expand All @@ -63,7 +65,7 @@ private Defaults() {
"transport")
)
),
new PlatformConfiguration(
new Platform(
"hive",
Language.JAVA,
HiveWrapperGenerator.class,
Expand All @@ -76,7 +78,7 @@ private Defaults() {
"transport")
)
),
new PlatformConfiguration(
new Platform(
"spark",
Language.SCALA,
SparkWrapperGenerator.class,
Expand All @@ -92,9 +94,9 @@ private Defaults() {
)
);

private static DependencyConfiguration getDependencyConfiguration(DependencyConfigurationName configurationName,
String moduleCoordinate, String platform) {
private static DependencyConfiguration getDependencyConfiguration(DependencyConfigurationType configurationName,
String module, String platform) {
return new DependencyConfiguration(configurationName,
moduleCoordinate + ":" + DEFAULT_VERSIONS.getProperty(platform + "-version"));
module + ":" + DEFAULT_VERSIONS.getProperty(platform + "-version"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@
* In the future can expand to incorporate exclude rules, dependency substitutions, etc.
*/
public class DependencyConfiguration {
private DependencyConfigurationName _configurationName;
private DependencyConfigurationType _configurationType;
private String _dependencyString;

public DependencyConfiguration(DependencyConfigurationName configurationName, String dependencyString) {
_configurationName = configurationName;
public DependencyConfiguration(DependencyConfigurationType configurationType, String dependencyString) {
_configurationType = configurationType;
_dependencyString = dependencyString;
}

public DependencyConfigurationName getConfigurationName() {
return _configurationName;
public DependencyConfigurationType getConfigurationType() {
return _configurationType;
}

public String getDependencyString() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
package com.linkedin.transport.plugin;

public enum DependencyConfigurationName {
public enum DependencyConfigurationType {
ANNOTATION_PROCESSOR,
COMPILE_ONLY,
IMPLEMENTATION,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@
/**
* Represents the information required to configure a given platform inside the {@link TransportPlugin}
*/
public class PlatformConfiguration {
public class Platform {

private final String _name;
private final Language _language;
private final Class<? extends WrapperGenerator> _wrapperGeneratorClass;
private final List<DependencyConfiguration> _defaultWrapperDependencies;
private final List<DependencyConfiguration> _defaultTestDependencies;

public PlatformConfiguration(String name, Language language, Class<? extends WrapperGenerator> wrapperGeneratorClass,
public Platform(String name, Language language, Class<? extends WrapperGenerator> wrapperGeneratorClass,
List<DependencyConfiguration> defaultWrapperDependencies, List<DependencyConfiguration> defaultTestDependencies) {
_name = name;
_language = language;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ static SourceDirectorySet getSourceDirectorySet(SourceSet sourceSet, Language la
}

private static Configuration getConfigurationForSourceSet(Project project, SourceSet sourceSet,
DependencyConfigurationName configurationName) {
DependencyConfigurationType configurationName) {
return project.getConfigurations().getByName(getConfigurationNameForSourceSet(sourceSet, configurationName));
}

private static String getConfigurationNameForSourceSet(SourceSet sourceSet,
DependencyConfigurationName configurationName) {
DependencyConfigurationType configurationName) {
final String configName;
switch (configurationName) {
case ANNOTATION_PROCESSOR:
Expand Down Expand Up @@ -87,7 +87,7 @@ static void addDependenciesToConfiguration(Project project, Configuration config
*/
static void addDependencyToSourceSet(Project project, SourceSet sourceSet, DependencyConfiguration dependency) {
addDependencyToConfiguration(project,
SourceSetUtils.getConfigurationForSourceSet(project, sourceSet, dependency.getConfigurationName()),
SourceSetUtils.getConfigurationForSourceSet(project, sourceSet, dependency.getConfigurationType()),
dependency.getDependencyString());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,19 @@ private void configureBaseSourceSets(Project project, SourceSet mainSourceSet, S
/**
* Configures SourceSets, dependencies and tasks related to each Transport UDF platform
*/
private void configurePlatform(Project project, PlatformConfiguration platformConfiguration,
private void configurePlatform(Project project, Platform platform,
SourceSet mainSourceSet, SourceSet testSourceSet) {
SourceSet sourceSet = configureSourceSet(project, platformConfiguration, mainSourceSet);
SourceSet sourceSet = configureSourceSet(project, platform, mainSourceSet);
TaskProvider<GenerateWrappersTask> generateWrappersTask =
configureGenerateWrappersTask(project, platformConfiguration, mainSourceSet, sourceSet);
configureGenerateWrappersTask(project, platform, mainSourceSet, sourceSet);
// TODO: shade and package into Jar
// Add Transport tasks to build task dependencies
project.getTasks().named(LifecycleBasePlugin.BUILD_TASK_NAME).configure(task -> {
// TODO: Replace this task with the shaded jar tasks once we create them
task.dependsOn(sourceSet.getClassesTaskName());
});

TaskProvider<Test> testTask = configureTestTask(project, platformConfiguration, mainSourceSet, testSourceSet);
TaskProvider<Test> testTask = configureTestTask(project, platform, mainSourceSet, testSourceSet);
project.getTasks().named(LifecycleBasePlugin.CHECK_TASK_NAME).configure(task -> task.dependsOn(testTask));
}

Expand All @@ -81,28 +81,26 @@ private void configurePlatform(Project project, PlatformConfiguration platformCo
* configurations and configures the default dependencies required for compilation and runtime of the wrapper
* SourceSet
*/
private SourceSet configureSourceSet(Project project, PlatformConfiguration platformConfiguration,
private SourceSet configureSourceSet(Project project, Platform platform,
SourceSet mainSourceSet) {
JavaPluginConvention javaConvention = project.getConvention().getPlugin(JavaPluginConvention.class);
Path platformBaseDir =
project.getBuildDir().toPath().resolve(Paths.get("generatedWrappers", platformConfiguration.getName()));
project.getBuildDir().toPath().resolve(Paths.get("generatedWrappers", platform.getName()));
Path wrapperSourceOutputDir = platformBaseDir.resolve("sources");
Path wrapperResourceOutputDir = platformBaseDir.resolve("resources");

return javaConvention.getSourceSets().create(platformConfiguration.getName(), sourceSet -> {
return javaConvention.getSourceSets().create(platform.getName(), sourceSet -> {
/*
Creates a SourceSet and set the source directories. E.g.
presto {
java.srcDirs = ["${buildDir}/generatedWrappers/sources"]
resources.srcDirs = ["${buildDir}/generatedWrappers/resources"]
compileClasspath += sourceSets.main.output
}
*/
SourceSetUtils.getSourceDirectorySet(sourceSet, platformConfiguration.getLanguage()).setSrcDirs(
SourceSetUtils.getSourceDirectorySet(sourceSet, platform.getLanguage()).setSrcDirs(
ImmutableList.of(wrapperSourceOutputDir));
sourceSet.getResources().setSrcDirs(ImmutableList.of(wrapperResourceOutputDir));
sourceSet.setCompileClasspath(project.files(sourceSet.getCompileClasspath(), mainSourceSet.getOutput()));

/*
Sets up the configuration for the wrapper SourceSet. E.g.
Expand All @@ -121,20 +119,24 @@ private SourceSet configureSourceSet(Project project, PlatformConfiguration plat
Adds default dependency config for Presto
dependencies {
prestoImplementation sourceSets.main.output
prestoImplementation 'com.linkedin.transport:transportable-udfs-presto:$version'
prestoCompileOnly 'com.facebook.presto:presto-main:$version'
}
*/
SourceSetUtils.addDependencyToConfiguration(project,
project.getConfigurations().getByName(sourceSet.getImplementationConfigurationName()),
mainSourceSet.getOutput());
SourceSetUtils.addDependenciesToSourceSet(project, sourceSet,
platformConfiguration.getDefaultWrapperDependencies());
platform.getDefaultWrapperDependencies());
});
}

/**
* Creates and configures a task to generate UDF wrappers for a given platform
*/
private TaskProvider<GenerateWrappersTask> configureGenerateWrappersTask(Project project,
PlatformConfiguration platformConfiguration, SourceSet inputSourceSet, SourceSet outputSourceSet) {
Platform platform, SourceSet inputSourceSet, SourceSet outputSourceSet) {

/*
Example generateWrapper task for Presto
Expand All @@ -151,21 +153,21 @@ private TaskProvider<GenerateWrappersTask> configureGenerateWrappersTask(Project
*/
String taskName = outputSourceSet.getTaskName("generate", "Wrappers");
File sourcesOutputDir =
SourceSetUtils.getSourceDirectorySet(outputSourceSet, platformConfiguration.getLanguage())
SourceSetUtils.getSourceDirectorySet(outputSourceSet, platform.getLanguage())
.getSrcDirs().iterator().next();
File resourcesOutputDir = outputSourceSet.getResources().getSrcDirs().iterator().next();

TaskProvider<GenerateWrappersTask>
generateWrappersTask = project.getTasks().register(taskName, GenerateWrappersTask.class, task -> {
task.setDescription("Generates Transport UDF wrappers for " + platformConfiguration.getName());
task.getGeneratorClass().set(platformConfiguration.getWrapperGeneratorClass().getName());
task.setDescription("Generates Transport UDF wrappers for " + platform.getName());
task.getGeneratorClass().set(platform.getWrapperGeneratorClass().getName());
task.getInputClassesDirs().set(inputSourceSet.getOutput().getClassesDirs());
task.getSourcesOutputDir().set(sourcesOutputDir);
task.getResourcesOutputDir().set(resourcesOutputDir);
task.dependsOn(project.getTasks().named(inputSourceSet.getClassesTaskName()));
});

project.getTasks().named(outputSourceSet.getCompileTaskName(platformConfiguration.getLanguage().toString()))
project.getTasks().named(outputSourceSet.getCompileTaskName(platform.getLanguage().toString()))
.configure(task -> task.dependsOn(generateWrappersTask));

return generateWrappersTask;
Expand All @@ -174,7 +176,7 @@ private TaskProvider<GenerateWrappersTask> configureGenerateWrappersTask(Project
/**
* Creates and configures a task to run tests written using the Unified Testing Framework against a given platform
*/
private TaskProvider<Test> configureTestTask(Project project, PlatformConfiguration platformConfiguration,
private TaskProvider<Test> configureTestTask(Project project, Platform platform,
SourceSet mainSourceSet, SourceSet testSourceSet) {

/*
Expand All @@ -192,14 +194,14 @@ private TaskProvider<Test> configureTestTask(Project project, PlatformConfigurat
}
*/
Configuration testClasspath =
project.getConfigurations().create(platformConfiguration.getName() + "TestClasspath", config ->
project.getConfigurations().create(platform.getName() + "TestClasspath", config ->
config.extendsFrom(
project.getConfigurations().getByName(testSourceSet.getImplementationConfigurationName()))
);
SourceSetUtils.addDependencyToConfiguration(project, testClasspath, mainSourceSet.getOutput());
SourceSetUtils.addDependencyToConfiguration(project, testClasspath, testSourceSet.getOutput());
SourceSetUtils.addDependenciesToConfiguration(project, testClasspath,
platformConfiguration.getDefaultTestDependencies());
platform.getDefaultTestDependencies());

/*
Creates the test task for the platform. E.g. For Presto,
Expand All @@ -213,9 +215,9 @@ task prestoTest(type: Test, dependsOn: test) {
}
*/

return project.getTasks().register(platformConfiguration.getName() + "Test", Test.class, task -> {
return project.getTasks().register(platform.getName() + "Test", Test.class, task -> {
task.setGroup(LifecycleBasePlugin.VERIFICATION_GROUP);
task.setDescription("Runs Transport UDF tests on " + platformConfiguration.getName());
task.setDescription("Runs Transport UDF tests on " + platform.getName());
task.setTestClassesDirs(testSourceSet.getOutput().getClassesDirs());
task.setClasspath(testClasspath);
task.useTestNG();
Expand Down

0 comments on commit a91bee5

Please sign in to comment.