-
Notifications
You must be signed in to change notification settings - Fork 467
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
[WFCORE-7035]: YAML extension doesn't support ParallelBoot. #6225
Conversation
742b7a4
to
9fa29d9
Compare
Core -> Full Integration Build 14010 outcome was FAILURE using a merge of 9fa29d9 Failed tests
|
Core -> WildFly Preview Integration Build 14083 outcome was FAILURE using a merge of 9fa29d9 Failed tests
|
controller/src/main/java/org/jboss/as/controller/AbstractOperationContext.java
Show resolved
Hide resolved
@@ -1237,6 +1240,9 @@ public final void reloadRequired() { | |||
|
|||
@Override | |||
public final void restartRequired() { | |||
if (isBooting()) { | |||
return; |
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 about debug logging.
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.
Can you add the debug logging?
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.
This still doesn't have the debug logging.
...oller/src/main/java/org/jboss/as/controller/persistence/yaml/YamlConfigurationExtension.java
Show resolved
Hide resolved
...oller/src/main/java/org/jboss/as/controller/persistence/yaml/YamlConfigurationExtension.java
Outdated
Show resolved
Hide resolved
...oller/src/main/java/org/jboss/as/controller/persistence/yaml/YamlConfigurationExtension.java
Show resolved
Hide resolved
292c5d2
to
d697170
Compare
…rtRequired on boot. * AbstractOperationContext is ignoring reloadRequired() and restartRequired() if isBooting() is returning true. Jira: https://issues.redhat.com/browse/WFCORE-7038 Signed-off-by: Emmanuel Hugonnet <ehugonne@redhat.com>
Core -> Full Integration Build 14395 outcome was FAILURE using a merge of 635ff97 Failed tests
|
Core -> Full Integration Build 14099 outcome was FAILURE using a merge of 635ff97 Failed tests
|
32f397c
to
4f1149a
Compare
Core -> Full Integration Build 14100 outcome was FAILURE using a merge of 4f1149a Failed tests
|
4f1149a
to
cbb23a5
Compare
@bstansberry I have simplified the code and added testing coverage |
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.
I understand this better now. I've got a few comments but I think it's basically good.
@@ -1237,6 +1240,9 @@ public final void reloadRequired() { | |||
|
|||
@Override | |||
public final void restartRequired() { | |||
if (isBooting()) { | |||
return; |
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.
Can you add the debug logging?
controller/src/main/java/org/jboss/as/controller/ParsedBootOp.java
Outdated
Show resolved
Hide resolved
...oller/src/main/java/org/jboss/as/controller/persistence/yaml/YamlConfigurationExtension.java
Show resolved
Hide resolved
Core -> WildFly Preview Integration Build 14182 outcome was FAILURE using a merge of cbb23a5 Failed tests
|
cbb23a5
to
d156f8e
Compare
@bstansberry I've taken your point into account. I also changed the algorithm a bit (again) since the ParrallelBootOp might have been changed by the remove operation so we should only use the 'final' one to update the list of child operations. |
Core -> WildFly Preview Integration Build 14183 outcome was FAILURE using a merge of d156f8e |
Core -> Full Integration Build 14402 outcome was FAILURE using a merge of d156f8e |
Core -> Full Integration Build 14104 outcome was FAILURE using a merge of d156f8e |
3521a7b
to
0c361fd
Compare
Core -> Full Integration Build 14403 outcome was FAILURE using a merge of 0c361fd |
* Adding YAML subsystem operations to the ParallelOperationHandler. * Adding more tests to cover that use-case. Jira: https://issues.redhat.com/browse/WFCORE-7035 Signed-off-by: Emmanuel Hugonnet <ehugonne@redhat.com>
0c361fd
to
2e11a98
Compare
Core -> Full Integration Build 14106 outcome was UNKNOWN using a merge of 2e11a98 |
Core -> WildFly Preview Integration Build 14185 outcome was UNKNOWN using a merge of 2e11a98 |
/retest please |
Core -> WildFly Preview Integration Build 14186 outcome was FAILURE using a merge of 2e11a98 |
Core -> Full Integration Build 14107 outcome was FAILURE using a merge of 2e11a98 |
Core -> WildFly Preview Integration Build 14187 outcome was FAILURE using a merge of 2e11a98 |
Core -> Full Integration Build 14108 outcome was FAILURE using a merge of 2e11a98 |
Core -> Full Integration Build 14405 outcome was FAILURE using a merge of 2e11a98 |
Core -> Full Integration Build 14404 outcome was FAILURE using a merge of 2e11a98 Failed tests
|
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.
@ehsavoie I left a comment about debug logging, but unless something goes wrong please handle this with a follow-up PR, as I'd prefer to get this merged as soon as CI finishes.
@@ -1237,6 +1240,9 @@ public final void reloadRequired() { | |||
|
|||
@Override | |||
public final void restartRequired() { | |||
if (isBooting()) { | |||
return; |
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.
This still doesn't have the debug logging.
Thanks @ehsavoie |
@bstansberry I created https://issues.redhat.com/browse/WFCORE-7088 to track your comment |
Adding YAML subsystem operations to the ParallelOperationHandler.
Jira: https://issues.redhat.com/browse/WFCORE-7035