Async step timeouts#1834
Conversation
vkalapov
force-pushed
the
async-step-timeouts
branch
from
51adc0c to
f3efdb1
Compare
|
|
|
||
| @Override | ||
| public Duration getTimeout(ProcessContext context) { | ||
| return calculateTimeout(context, TimeoutType.BIND_SERVICE); |
There was a problem hiding this comment.
why checking a service key operation is using BIND_SERVICE instead of CREATE_SERVICE_KEY
|
|
||
| @Named("globalTimeoutSettingStep") | ||
| @Scope(BeanDefinition.SCOPE_PROTOTYPE) | ||
| public class GlobalTimeoutSettingStep extends SyncFlowableStep { |
There was a problem hiding this comment.
Use constructor injection
| return timeoutType.isModuleScoped() ? moduleTimeoutSources : serviceTimeoutSources; | ||
| } | ||
|
|
||
| public DeploymentDescriptor getDeploymentDescriptor(ProcessContext context, StepLogger stepLogger) { |
There was a problem hiding this comment.
Why TimeoutValueResolved has a method for getting the DeploymentDescriptor from context. This breaks single responsibility principle
| if (!timeoutType.isServiceScoped()) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Why is this check needed for?
| String timeoutSeconds = timeout != null ? String.valueOf(timeout.toSeconds()) : "null"; | ||
| getStepLogger().debug(Messages.TIMEOUT_0_EQUALS_1_SECONDS_FROM_2, |
There was a problem hiding this comment.
Can we say the parameter is not set instead of logging 'null' strings?
| import org.cloudfoundry.multiapps.mta.model.Resource; | ||
|
|
||
| @Named | ||
| public class TimeoutServiceResourceNameResolver { |
There was a problem hiding this comment.
Is this used beyond tests?
| private interface TimeoutSource { | ||
| Optional<TimeoutResolution> resolve(ProcessContext context, TimeoutType timeoutType, StepLogger logger); |
There was a problem hiding this comment.
Why TimeoutSource has a resolve method? Rename to some resolver?
| return DurationUtil.parseDuration(timeout, timeoutParameterName, maxAllowedValue); | ||
| } | ||
|
|
||
| private Optional<Module> findModuleByAppName(DeploymentDescriptor descriptor, String appName) { |
There was a problem hiding this comment.
The module name and the app-name might not always match. Can we reuse some already existing method util instead of having another similar method?
| public record TimeoutResolution(Duration timeout, String parameterName) { | ||
| } | ||
|
|
||
| private final List<TimeoutSource> moduleTimeoutSources = List.of( |
There was a problem hiding this comment.
Isn't the whole logic a bit over engineered with all these lambdas and records?
| .filter(type -> originalParameters.containsKey(type.getProcessVariable() | ||
| .getName())) | ||
| .forEach(type -> targetParameters.put(type.getOperationParamsFlag() |
There was a problem hiding this comment.
What is the point of setting these variables to true if they are hardcoded in the enum while their default value is false?
2 participants
LMCROSSITXSADEPLOY-3322