Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow newer java versions (LTS and current), correct interactive logic #6983

Merged
merged 3 commits into from Jul 1, 2020

Conversation

donsizemore
Copy link
Contributor

What this PR does / why we need it:

In testing/using dataverse-ansible, @pallinger correctly pointed out that 1) install.py requires openjdk 1.8, and 2) somehow when openjdk-1.8.0 is specified install.py carries on in ansible without complaint, but when openjdk-11 is specified it hangs waiting for a password, when --noninteractive is passed.

This PR modifies install.py to allow OpenJDK 1.8 (long-term RedHat support), 11 (OpenJDK LTS) and 14 (current OpenJDK, not recommended), and corrects the noninteractive password logic per @landreev

Which issue(s) this PR closes:

Closes #6982

Special notes for your reviewer: none come to mind

Suggestions on how to test this: re-run install.py with and without --noninteractive, while openjdks 1.8.0, 11 and maybe 14 are set as default

Does this PR introduce a user interface change? If mockups are available, please link/include them here: no

Is there a release notes update needed for this change?: no

Additional documentation: none.

This can definitely wait until after the DCM.

@coveralls
Copy link

coveralls commented Jun 15, 2020

Coverage Status

Coverage remained the same at 19.62% when pulling 79c5dfa on OdumInstitute:6982_install_py_java_check into c79ce40 on IQSS:develop.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

I'm not so sure if makes sense to support Java 14. Details inline.

if not re.search("1.8", java_version):
sys.exit("Dataverse requires Java 1.8. Please install it, or make sure it's in your PATH, and try again")
if not re.search('(1.8|11|14)', java_version):
sys.exit("Dataverse requires Java 1.8, 11 or 14. Please make sure it's in your PATH, and try again.")
Copy link
Member

Choose a reason for hiding this comment

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

Supporting Java 14 feels a little arbitrary. It's not an LTS release. It just happens to be the current release. My concern is that if we really want to support the latest release of Java, we're going to have to edit this line every six months. I'd be happier with just putting the next LTS release in there, Java 17, which is due out September 2021. Or we could remove Java 14.

Here's a table from Wikipedia that shows all the recent Java releases:

Screen Shot 2020-06-15 at 2 10 42 PM

Copy link
Member

Choose a reason for hiding this comment

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

@donsizemore we talked about this a bit in Slack. Can you please remove Java 14 and only support 8 and 11? Thanks!

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

Let's only allow LTS releases for now: Java 8 and 11.

if not re.search("1.8", java_version):
sys.exit("Dataverse requires Java 1.8. Please install it, or make sure it's in your PATH, and try again")
if not re.search('(1.8|11|14)', java_version):
sys.exit("Dataverse requires Java 1.8, 11 or 14. Please make sure it's in your PATH, and try again.")
Copy link
Member

Choose a reason for hiding this comment

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

@donsizemore we talked about this a bit in Slack. Can you please remove Java 14 and only support 8 and 11? Thanks!

@pdurbin pdurbin moved this from Code Review 🦁 to Community Dev 💻❤️ in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) Jun 15, 2020
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from Community Dev 💻❤️ to QA 🔎✅ Jun 15, 2020
@kcondon kcondon self-assigned this Jun 16, 2020
@kcondon kcondon merged commit 95510a3 into IQSS:develop Jul 1, 2020
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from QA 🔎✅ to Done 🚀 Jul 1, 2020
@djbrooke djbrooke added this to the Dataverse 5 milestone Jul 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

install.py requires Java 1.8
5 participants