WFLY-10784 not fail WeldStartCompletionService on optional components #11481
Conversation
There was a problem hiding this comment.
@kudrevatykh first and foremost, I don't understand what is the case where current solution fails (even after reading the JIRA issue). Can you provide a reproducer, detailed description, or, better still, add an automated test?
Secondly, the PR uses deprecated methods from ServiceBuilder which rely on DependencyType and according to javadoc - "Optional dependencies are unsafe and should not be used.". So this isn't a way to go.
We first need to see what's the actual problem and have a way of reproducing it, then we can jump to solutions. That being said, we will likely need to update WeldDeploymentProcessor anyway to remove usage of deprecated classes.
| for (DeploymentUnit sub : subDeployments) { | ||
| weldStartCompletionServiceBuilder.addDependencies(getComponentStartServiceNames(sub)); | ||
| getComponentStartServiceNames(sub).entrySet().forEach(e->weldStartCompletionServiceBuilder.addDependencies(e.getKey(), e.getValue())); |
There was a problem hiding this comment.
ServiceBuilder.addDependencies() is deprecated and even says that optional dependencies are unsafe and should not be used.
There was a problem hiding this comment.
Can you provide recipe how to depend on component when it can not start in any moment?
There was a problem hiding this comment.
I don't know if it is possible at all. With current setup, Weld can do some cleanup operation allowing us to lower the footprint. If we cannot rely on all components started, then we cannot do any cleanup. It also seems that org.jboss.msc.service.ServiceTarget doesn't support your desired approach at all.
Can you please provide a reproducer for your problem (via JIRA - https://issues.jboss.org/browse/WFLY-10784)? If we had that, we could start fiddling with it and see what we can do.
There was a problem hiding this comment.
I added commit with reproducer, but can't run any test in wildlfy source tree, only in separate project
| .add(ConfigurationKey.NON_PORTABLE_MODE.get(), nonPortableMode) | ||
| .add(ConfigurationKey.ALLOW_OPTIMIZED_CLEANUP.get(), true) | ||
| .build(); | ||
| .add(ConfigurationKey.NON_PORTABLE_MODE.get(), nonPortableMode) |
|
@kudrevatykh Adding the fixme label until you have addressed @manovotn 's concerns. @mention me once it is all in order |
Not fail WeldStartCompletionService when optional components are not started
EJB service and WAR can be deployed when then independent modules, but in EAR can't be deployed
kudrevatykh
force-pushed
the
bugfix/WFLY-10784
branch
from
6dcd6af
to
99c3a85
Compare
|
@kabir please close this PR. I will raise a different one with the approach Brian suggested on JIRA. |
Thanks for submitting your Pull Request!
Please make sure your PR meets the following requirements:
[WFLY-XYZ] SubjectorWFLY-XYZ SubjectFor bigger changes, major and minor component upgrades make sure your PR also meets following requirements:
For new features ensure as well:
If you are not an active contributor of the WildFly project you can request sponsorship by one of the members to help guide you through the process.