dataverse icon indicating copy to clipboard operation
dataverse copied to clipboard

7980 enhanced dsd

Open poikilotherm opened this issue 2 years ago • 1 comments

What this PR does / why we need it: Create real defaults for the database connection, enable configuring advanced settings, add docs.

Which issue(s) this PR closes:

Closes #7980

Special notes for your reviewer:

  • The release docs of older releases do not need to be changed. The statements on how to upgrade etc are working just fine. Only in a few occasions people might have hit the regression.
  • This PR replaces #7983.
  • This PR requires people to update to Payara 5.2022.2 at least. Insiders know about 5.2022.3 on special delivery.

Suggestions on how to test this:

  1. Remove the properties from domain.xml. Should still work (if you had used the defaults in there)
  2. Try setting the JDBC logging to enabled via system property: asadmin create-system-properties "dataverse.db.log-jdbc-calls=true"

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

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

Additional documentation: None.

poikilotherm avatar Aug 11 '22 21:08 poikilotherm

Coverage Status

Coverage: 19.997%. Remained the same when pulling 8106ddf9b57d0830b38f778875addcb9a5d2ceb2 on poikilotherm:7980-enhanced-dsd into f63f0e85bfb8f2e8526551b260744f17f2d99915 on IQSS:develop.

coveralls avatar Aug 11 '22 21:08 coveralls

sizing: Waiting on Oliver input. He is out today.

mreekie avatar Jan 04 '23 16:01 mreekie

Phil is going to talk with Oliver to see if this can be lowered in priority in the community column

mreekie avatar Jan 05 '23 16:01 mreekie

I've just merged latest develop. This is a more or less trivial change, as there isn't much to test. The functionality is provided by Payara, so IMHO there is no point in testing that. One might test setting a setting, but at this point IMHO we should be confident enough that Payara handles these variable replacements well.

That said, yes, it may move down the column. I have no urgent business requiring this ATM.

poikilotherm avatar Jan 06 '23 13:01 poikilotherm

That said, yes, it may move down the column. I have no urgent business requiring this ATM.

You know what? Based on comments by @landreev and @donsizemore in the issue regarding the usefulness of sslmode=require I'm fine with keeping this where it is priority-wise. I'll throw a 3 on it for size. I hope that's big enough.

By the way, there's a failing test but I suspect it's unrelated:

java.lang.AssertionError: 
Expected status code <200> doesn't match actual status code <403>.

	at edu.harvard.iq.dataverse.api.FilesIT.test_008_ReplaceFileAlreadyDeleted(FilesIT.java:846)

pdurbin avatar Jan 06 '23 19:01 pdurbin

Pdurbin reviewed and sized. Moving to sprint ready column.

mreekie avatar Jan 09 '23 13:01 mreekie

@poikilotherm Can you refresh from develop? Getting flyway errors on deployment. Thanks!

kcondon avatar Jan 23 '23 19:01 kcondon

@kcondon done! Refreshed! Ahhh.

pdurbin avatar Jan 23 '23 21:01 pdurbin