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.

Sign up for GitHub

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

Jump to bottom

Upgrade Narayana to 5.0.4.Final #7036

Merged
merged 3 commits into from
Dec 15, 2014
Merged

Upgrade Narayana to 5.0.4.Final #7036

merged 3 commits into from
Dec 15, 2014

Conversation

tomjenkinson
Copy link
Contributor

@wildfly-ci
Copy link

Linux with security manager Build 635 is now running using a merge of fe41ce2

@wildfly-ci
Copy link

Linux Build 5650 is now running using a merge of fe41ce2

@wildfly-ci
Copy link

Windows Build 761 is now running using a merge of fe41ce2

@wildfly-ci
Copy link

Linux with security manager Build 635 outcome was SUCCESS using a merge of fe41ce2
Summary: Tests passed: 797, ignored: 370 Build time: 0:06:20

@wildfly-ci
Copy link

Windows Build 761 outcome was SUCCESS using a merge of fe41ce2
Summary: Tests passed: 3017, ignored: 365 Build time: 0:51:26

@wildfly-ci
Copy link

Linux Build 5650 outcome was SUCCESS using a merge of fe41ce2
Summary: Tests passed: 3016, ignored: 365, muted: 1 Build time: 0:54:37

bstansberry
bstansberry reviewed Dec 12, 2014
View reviewed changes
transactions/src/main/java/org/jboss/as/txn/subsystem/TransactionSubsystemAdd.java
@@ -234,6 +234,7 @@ protected void execute(final DeploymentProcessorTarget processorTarget) {
processorTarget.addDeploymentProcessor(TransactionExtension.SUBSYSTEM_NAME, Phase.POST_MODULE, Phase.POST_MODULE_TRANSACTIONS_EE_CONCURRENCY, new EEConcurrencyContextHandleFactoryProcessor());
processorTarget.addDeploymentProcessor(TransactionExtension.SUBSYSTEM_NAME, Phase.INSTALL, Phase.INSTALL_TRANSACTION_BINDINGS, new TransactionJndiBindingProcessor());
processorTarget.addDeploymentProcessor(TransactionExtension.SUBSYSTEM_NAME, Phase.DEPENDENCIES, Phase.DEPENDENCIES_TRANSACTIONS, new TransactionDependenciesProcessor());
processorTarget.addDeploymentProcessor(TransactionExtension.SUBSYSTEM_NAME, Phase.DEPENDENCIES, Phase.DEPENDENCIES_XTS, new CompensationsDependenciesDeploymentProcessor());
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this relate to XTS? Or is this more of a clever workaround to the big hassle caused by having the Phase class in WFCORE?

Copy link
Contributor

Choose a reason for hiding this comment

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

This code used to live in the XTS subsytem, but doesn't actually belong there as you can use compensating transactions without XTS (in the case where you don't need a distributed transaction). I think during the move, I forgot to change Phase.DEPENDENCIES_XTS.

I think we need to change Phase.DEPENDENCIES_XTS to Phase.DEPENDENCIES_TRANSACTIONS.

I'm on holiday today so can't test this change. Maybe @tomjenkinson can give it a go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi,

I have made the change and it works fine now. the update has been squashed into this PR so the commit is now 459fa8d.

Thanks,
Tom

@bstansberry bstansberry added the ready-for-merge Only for use by those with merge permissions! label Dec 12, 2014
@wildfly-ci
Copy link

Linux with security manager Build 645 is now running using a merge of 459fa8d

@wildfly-ci
Copy link

Windows Build 769 is now running using a merge of 459fa8d

@wildfly-ci
Copy link

Linux Build 5658 is now running using a merge of 459fa8d

@wildfly-ci
Copy link

Linux with security manager Build 645 outcome was FAILURE using a merge of 459fa8d
Summary: Tests failed: 1 (1 new), passed: 796, ignored: 370 Build time: 0:09:15

Build problems:

Failed tests detected

Failed tests

org.jboss.as.test.smoke.webservices.WSTestCase.testManagementDescriptionMetrics: java.lang.AssertionError: null
    at org.junit.Assert.fail(Assert.java:86)
    at org.junit.Assert.assertTrue(Assert.java:41)

@wildfly-ci
Copy link

Linux Build 5658 outcome was SUCCESS using a merge of 459fa8d
Summary: Tests passed: 3017, ignored: 365 Build time: 0:54:17

@wildfly-ci
Copy link

Windows Build 769 outcome was SUCCESS using a merge of 459fa8d
Summary: Tests passed: 3017, ignored: 365 Build time: 1:03:55

@tomjenkinson
Copy link
Contributor Author

Hi @bstansberry , I don't see how my change can have caused the WSTestCase to fail, do you think it would be possible to merge this one please? Please do let me know if there is anything you need from our side.

@bstansberry
Copy link
Contributor

@tomjenkinson that test fails frequently; it's unrelated to this PR.

I planned to merge this Friday after I added the "Ready to merge" label, but our CI servers were unavailable. Now it goes in...

bstansberry added a commit that referenced this pull request Dec 15, 2014
@bstansberry
Merge pull request #7036 from tomjenkinson/WFLY-4175
77ece4d 
Upgrade Narayana to 5.0.4.Final
@bstansberry bstansberry merged commit 77ece4d into wildfly:master Dec 15, 2014
@tomjenkinson tomjenkinson deleted the WFLY-4175 branch April 21, 2017 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
ready-for-merge Only for use by those with merge permissions!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tomjenkinson @wildfly-ci @bstansberry @paulrobinson