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
WFLY-381 Add undertow support to mod_cluster subsystem. #4691
Conversation
Triggering build using a merge of c5417eb on branch master: |
Build 7551 is now running using a merge of c5417eb on branch master: |
Build 7551 outcome was FAILURE using a merge of c5417eb on branch master: |
Looks like we forgot to add the compatibility module, fix: |
Well, more fundamentally, I never ran this portion of the testsuite... ;) However, I don't think we should have to add a compatibility module, since it is never referenced by any other module that would ever cause a problem. Rather, there are a host of config files in these testsuites that need to be modified to use the new module name. |
Actually the testsuite tests with already released set of configurations (e.g. 7.0.2 shipped standalone-ha.xml) to test backwards compatibility. Those should never be changed. So I really think we want to keep a compatibility module around (it really just "links" the old name to new name and exports services so its really trivial). I have rerun few tests locally with this patch rhusar/wildfly@b105a78 and they passed, I have started a whole run #4694 too see if it fixes all. |
Ah, of course, the module is ultimately referenced by name as <extension module="..."/> in the configs, so forget what I said earlier... |
Triggering build using a merge of 769d2e3 on branch master: |
Build 7557 is now running using a merge of 769d2e3 on branch master: |
Yeah, it passes with that change now completely: #4694 (comment) |
Build 7557 outcome was FAILURE using a merge of 769d2e3 on branch master: |
One more thingy: pferraro#1 |
Triggering build using a merge of ed0a0ff on branch master: |
Build 7558 is now running using a merge of ed0a0ff on branch master: |
Build 7558 outcome was FAILURE using a merge of ed0a0ff on branch master: |
Um - I don't think that failure is related. Retest this please. |
Triggering build using a merge of ed0a0ff on branch master: |
Build 7559 is now running using a merge of ed0a0ff on branch master: |
Build 7559 outcome was SUCCESS using a merge of ed0a0ff on branch master: |
Huh, actually it doesnt work at all now. Here is a fix: rhusar@65234ca PR: pferraro#2
|
Triggering build using a merge of a9f4feffa6084e5ddbfa98b7c756fc5068cc9e3c on branch master: |
Build 7584 is now running using a merge of a9f4feffa6084e5ddbfa98b7c756fc5068cc9e3c on branch master: |
Build 7584 outcome was SUCCESS using a merge of a9f4feffa6084e5ddbfa98b7c756fc5068cc9e3c on branch master: |
This is gathering dust. Can someone review this please? |
@@ -76,12 +76,15 @@ | |||
*/ | |||
public class ModClusterExtension implements XMLStreamConstants, Extension { | |||
|
|||
public static final String SUBSYSTEM_NAME = "modcluster"; | |||
static final String LEGACY_SUBSYSTEM_NAME = "modcluster"; | |||
// Might it be possible to rename this subsystem?! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Damn.
</dynamic-load-provider> | ||
</mod-cluster-config> | ||
</subsystem> | ||
<socket-binding name="mod_cluster" port="0" multicast-address="224.0.1.105" multicast-port="23364"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. I notice in the testsuite you had to change tests to deal with this. Anyone else who writes tests against WF/EAP7 will have to waste time doing the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intention is to address https://issues.jboss.org/browse/WFLY-689
Merged. |
No description provided.