Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

SQOOP-3441 Prepare Sqoop for Java 11 support #81

Merged
merged 3 commits into from Jun 12, 2019
Merged

Conversation

fszabo2
Copy link
Contributor

@fszabo2 fszabo2 commented Jun 5, 2019

Contains the following minor fixes that came up when executing tests with java11:

  • Bumped gradle version from 4.9 to 5.3.1 (needed for jacoco plugin)
  • Fixed illegal access of constructor in a TestSqoopOptions
  • Removed the use of UrlClassLoader as it wasn't needed anyway (Accumulo mini cluster startup)

@szvasas
Copy link
Contributor

szvasas commented Jun 5, 2019

Hi Feró,

Thanks for submitting this improvement, unfortunately I don't have time to post a full review just had one comment on the Gradle upgrade.
Since we have the following wrapper task defined in build.gradle I think it would be good to maintain the version number in it:

wrapper {
    gradleVersion = '4.9'
}

Then running ./gradlew wrapper to make sure everything is fully updated to the version specified.
There are also some guidelines provided here: https://docs.gradle.org/current/userguide/gradle_wrapper.html#sec:upgrading_wrapper

(We could also remove the wrapper task and just run ./gradlew wrapper --gradle-version <version> in the future.)

Contains the following minor fixes that came up when executing tests with java11:
- Bumped gradle version from 4.9 to 5.3.1 (needed for jacoco plugin)
- Fixed illegal access of constructor in a TestSqoopOptions
- Removed the use of UrlClassLoader as it wasn't needed anyway (Accumulo mini cluster startup)
@fszabo2
Copy link
Contributor Author

fszabo2 commented Jun 11, 2019

Hi @szvasas,
Thanks for pointing this out!
I ran the command ./gradlew wrapper --gradle-version 5.3.1, and then force pushed as I also noticed a typo in the decription and wanted to correct that as well.

In any case, this is a proper upgrade now, albeit to 5.3.1 that is a deprecated version. I just realized this, I'll update the version to 5.4.1, if all tests pass that is the version we should use, it's the currently supported one.

Not sure why Oracle tests failed...

Bumped gradle version to 5.4.1
Copy link
Member

@ebogi ebogi left a comment

Choose a reason for hiding this comment

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

Hi Fero, many thanks for preparing Sqoop for JDK 11 support! Your change LGTM but could you please update documentation (e.g. COMPILING.adoc) too? Thanks!

Fixed Gradle version in documentation.
@fszabo2
Copy link
Contributor Author

fszabo2 commented Jun 12, 2019

Hi @ebogi,
Thanks for the review! I've updated the version in COMPILING.adoc, automatic tests should pass all the same.

Copy link
Member

@ebogi ebogi left a comment

Choose a reason for hiding this comment

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

Thanks for the doc update! Ship it!

@ebogi ebogi merged commit cee4ab1 into apache:trunk Jun 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants