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

IQSS/10557-fix-installer-pid-setup #10563

Conversation

qqmyers
Copy link
Member

@qqmyers qqmyers commented May 14, 2024

What this PR does / why we need it: This PR adds a missing command to set the default pid provider in the as-setup.sh file. It also adds a check in the ConfigChecker service to make sure one is set.

Which issue(s) this PR closes:

Closes #10557

Special notes for your reviewer:

Suggestions on how to test this: Run this installer (vs docker setup). Could also verify that after the PR, removing the default-provider setting causes starting payara to fail at the config checker stage with a warning in the log.

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

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

Additional documentation:

@qqmyers qqmyers added the Size: 3 A percentage of a sprint. 2.1 hours. label May 14, 2024
@qqmyers qqmyers added this to the 6.3 milestone May 14, 2024
@coveralls
Copy link

coveralls commented May 14, 2024

Coverage Status

coverage: 20.635% (+2.3%) from 18.333%
when pulling 2626050 on GlobalDataverseCommunityConsortium:IQSS/10557-fix_installer_fake_pid_setup
into ba209ad on IQSS:develop.

@pdurbin
Copy link
Member

pdurbin commented May 15, 2024

@qqmyers thanks for the PR!

As I was saying at standup and in Slack just now, we have reason to believe that people installing Dataverse 6.2 for the first time (non-upgrade) using traditional methods (non-Docker) will be unable to create datasets (Issue#10557). Thank you, Jim, for providing a fix in PR#10563. Assuming this is right, we should rebundle dvinstall.zip with the fixed script, indicate this at the top of the release notes, and send a message to the google group

@pdurbin
Copy link
Member

pdurbin commented May 16, 2024

As I explained at #10557 (comment) I can easily reproduce the bug. Accept all the installer defaults and you can't create a dataset.

I haven't tested the changes to the Java code in this PR but as mentioned on Slack I did test the new as-setup.sh script and it fixes the problem. I can create a dataset. I went ahead and uploaded a new dvinstall.zip to the release with the new script (I replaced it in place with zip dvinstall.zip dvinstall/as-setup.sh). And I added the following to the top of the 6.2 release notes:

Screenshot 2024-05-16 at 2 47 23 PM

Please feel free to edit further.

Oh, I renamed the old dvinstall.zip to sus.zip. We can probably delete it.

@sekmiller sekmiller self-assigned this Jun 25, 2024
Copy link
Contributor

@sekmiller sekmiller left a comment

Choose a reason for hiding this comment

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

Looks good. Moving on

@sekmiller sekmiller removed their assignment Jun 25, 2024
@landreev landreev self-assigned this Jun 27, 2024
@landreev
Copy link
Contributor

The PR looks good, the installer works; I was ready to merge, but noticed that Jenkins tests never ran on it. I don't think it's necessary, but figured I'd run it just in case, to make sure the config check works with the Jenkins environment.

@landreev landreev merged commit 43ee260 into IQSS:develop Jun 27, 2024
11 checks passed
@qqmyers qqmyers deleted the IQSS/10557-fix_installer_fake_pid_setup branch June 28, 2024 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: 3 A percentage of a sprint. 2.1 hours.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error when trying to create a new Dataset (NullPointerException)
5 participants