[WFCORE-5718] Remoting: add an ability to configure protocol used by … #5151
Conversation
github-actions
bot
added
the
deps-ok
yersan
added
Feature
|
There has been no activity on this PR for 45 days. It will be auto-closed after 90 days. |
| @@ -0,0 +1,174 @@ | |||
| /* | |||
| * JBoss, Home of Professional Open Source. | |||
| * Copyright 2015 Red Hat, Inc., and individual contributors | |||
There was a problem hiding this comment.
Fix year. We can't apply licence to the past.
| import static org.jboss.as.remoting.CommonAttributes.SOCKET_BINDING; | ||
|
|
||
| /** | ||
| * Parser for version 5.0 of the subsystem schema. |
There was a problem hiding this comment.
Says it's parser for 5 but class name is RemotingSubsystem60Parser
tadamski
force-pushed
the
WFCORE-5718
branch
from
9f2cc54 to
dbf6efe
Compare
|
@rhursar could you please take a look if the PR is ok now? |
|
There has been no activity on this PR for 45 days. It will be auto-closed after 90 days. |
|
@yersan I have updated https://issues.redhat.com/browse/EAP7-1672, created test plan, and fixed the proposal PR |
|
@tadamski, Feature fields (TP, PC, FI) are in an inconsistent state and the issue is still in "in Progress", we don't merge until it reaches the "Ready for merge" state. The Analysis Doc also mentions ejb-client, but the EJB client PR was closed. There is also a WFLY Jira linked which it seems should have been cloned to WFCORE. If there are no changes in WildFly, we don't need a WFLY Jira. Please clarify those points so we know how to continue with this one. |
|
@yersan I have set all those fields. From my perspective the issue and the tests are ready so I set the status to "Ready to merge". Regarding PRs, I usually prefer to open PRs one by one because the wildfly PR is guaranteed to fail without this one merged, but if necessary I can open three PRs now. |
|
Hi @tadamski , moving to "Ready to merge" requires additional previous steps, take a look at the Feature Development Process and get familiar with it. Essentially, your Feature Request doesn't have a QE assigned. The QE is the one that will merge the test plan and approve the Analysis doc. Once all the deliverables are in place and your team has done the pre-check, you can move it to "Ready to merge". Regarding the WildFly related PR, it will depend on your test plan and if your tests and verification need to run the CI with both PRs (WildFly and WildFly Core) in place. Apparently, since your feature involves other components, it is likely you want to test both to verify all changes and component upgrades work together. In addition, this feature seems it needs changes in other components, I see REM3-389 and EJBCLIENT-388 linked to the WildFly Jira, although these links are not in the analysis doc, they should be there for reference. These components should be in Final and their versions bumped in WildFly and WildFly Core before merging. |
|
thanks @yersan; I made a mistake of thinking that I don't need a QE contact when I'm a tester - I have asked Martin to assign somebody for this role. Regarding PRs, I have opened a PR in ejb-client. The REM3-389 PR is invalid - I have closed it and the related JIRA issue and I'm going to remove it from the links. |
|
There has been no activity on this PR for 45 days. It will be auto-closed after 90 days. |
|
/retest |
|
There has been no activity on this PR for 45 days. It will be auto-closed after 90 days. |
There was a problem hiding this comment.
Can we please have the schema bump and actual model changes in separate commits.
The git and GitHub APIs don't let us see the actual schema updates specific to this change without manual diffing, also if others need to make changes to this subsystem they can build on the schema bump commit without conflicting with this PR.
|
@darranl I have split the commit. Is that ok? |
| * | ||
| * @author <a href=mailto:[email protected]>Tomasz Adamski</a> | ||
| */ | ||
| class RemotingSubsystem60Parser extends RemotingSubsystem40Parser { |
There was a problem hiding this comment.
The addition of this class should also be in the first commit so we can see the changes made in the second commit. however isn't there a RemotingSubsystem50Parser this should be extending?
There was a problem hiding this comment.
@darranl Rebased. There is a empty 5.0 parser, but I changed the code so that it extends it.
|
/retest |
|
There has been no activity on this PR for 45 days. It will be auto-closed after 90 days. |
|
There has been no activity on this PR for 90 days and it has been closed automatically. |
|
There has been no activity on this PR for 45 days. It will be auto-closed after 90 days. |
|
There has been no activity on this PR for 90 days and it has been closed automatically. |
…the remoting connector
…the remoting connector
https://issues.redhat.com/browse/EAP7-1672
https://issues.redhat.com/browse/WFCORE-5718
https://issues.redhat.com/browse/WFLY-13828
This is a configuration fix that allows for easy configuration of feature required by EAP7-1672, which is using "remote+tls" protocol from ejb client. The protocol is supported by jboss-remoting but simply adding it to the configuration doesn't work out of the box. I have noticed that no matter what the connector configuration is on the server side, the connector is hardcoded to use
a connection provider associated with "remoting" protocol. This provider is configured differently than "remote+tls" one and this lead to inability to establish a connection. With this change customer has an ability to configure "remote+tls" on both client and the server and establish a connection using "always SSL" protocol.
@fl4via could you please review?