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

APPSERV-22 Remove Derby #4451

Merged
merged 17 commits into from
Feb 21, 2020
Merged

APPSERV-22 Remove Derby #4451

merged 17 commits into from
Feb 21, 2020

Conversation

MeroRai
Copy link
Member

@MeroRai MeroRai commented Jan 23, 2020

Description

We've bundled both the H2 and Derby databases when only one is required.

Important Info

Dependant PRs

Test suites executed

  • Quicklook
  • Java EE7 Samples
  • Java EE8 Samples
  • Payara Private Tests
  • Jakarta TCKs

Testing Environment

Zulu JDK 1.8_222 on Elementary OS 0.4.1 Loki with Maven 3.5.4>

@MeroRai MeroRai self-assigned this Jan 23, 2020
@Pandrex247 Pandrex247 added this to the 5.201 milestone Jan 24, 2020
Copy link
Contributor

@jbee jbee left a comment

Choose a reason for hiding this comment

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

Very impressive that you could pull this change off - but also very scary - the amount of files that needed to change and the level of duplication that is apparently going on in this old code.

Only have some minor comments.

As I am not knowledgeable on the role of derby in connection with the server I could only look at the changes made but I cannot say anything about changes that were not made but maybe should have been. Therefore I feel someone with a better idea on this should also take a look at this PR.

I am going to run suggested tests - if they come out clean I am happy with this PR as is.

Edit: Note to all other reviewers: Make sure to review with whitespace changes not being shown ;)

@@ -211,8 +206,6 @@ public void init(IBatchConfig batchConfig) throws BatchContainerServiceException
public CheckpointData getCheckpointData(CheckpointDataKey key) {
logger.entering(CLASSNAME, "getCheckpointData", key == null ? "<null>" : key);

tryObtainTableLock();
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this is not needed for H2 for some reason?

Copy link
Member Author

Choose a reason for hiding this comment

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

This was written specifically for Derby

Copy link
Contributor

@jbee jbee left a comment

Choose a reason for hiding this comment

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

Finished running build and tests. Got two failures in appserver/tests/payara-samples but I am pretty sure they are unrelated flaky tests.

@MarkWareham MarkWareham added the PR: DO NOT MERGE Don't merge PR until further notice label Feb 12, 2020
@MarkWareham
Copy link
Contributor

See Jira for details of DO NOT MERGE

@MeroRai
Copy link
Member Author

MeroRai commented Feb 13, 2020

Jenkins test please

1 similar comment
@MeroRai
Copy link
Member Author

MeroRai commented Feb 14, 2020

Jenkins test please

@MarkWareham MarkWareham removed the PR: DO NOT MERGE Don't merge PR until further notice label Feb 19, 2020
@MeroRai
Copy link
Member Author

MeroRai commented Feb 20, 2020

Jenkins test please

@MeroRai MeroRai merged commit 0d2e038 into payara:master Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants