Skip to content

Conversation

@jbaiera
Copy link
Member

@jbaiera jbaiera commented Oct 18, 2019

CI jobs run with the -Pdistro=hadoopYarn setting, which changes the Hadoop dependency at runtime to 2.7 instead of 2.2. This causes the CI jobs to fail because the SHA files for the dependency to not match the new dependencies. This PR switches the default hadoop distro used for building to be consistent with the one used in CI.

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

I left a couple of comments, I think this code could be simplified, or else a default: clause added to be explicit.

break

default:
case "hadoopStable":
Copy link
Member

Choose a reason for hiding this comment

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

We should add a default: case and throw an error, so we don't accidentally introduce leniency in the future

case "hadoopStable":
String version = project.hadoop22Version
project.rootProject.ext.hadoopVersion = version
project.rootProject.ext.hadoopClient = ["org.apache.hadoop:hadoop-client:$version"]
Copy link
Member

@dakrone dakrone Oct 24, 2019

Choose a reason for hiding this comment

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

There's no difference in the code of the two different switch paths other than the println, can we move the settings out? Unless we need to not set them in the 'default' case (in which case we should be explicit in that)

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM, hooboy I had to stare at project.hadoop2Version and project.hadoop22Version for a while to see the difference :)

@jbaiera jbaiera merged commit ab6c478 into elastic:master Oct 25, 2019
@jbaiera jbaiera deleted the fix-license-build-failures branch October 25, 2019 15:49
jbaiera added a commit that referenced this pull request May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants