wmedvede commented on code in PR #415:
URL:
https://github.com/apache/incubator-kie-kogito-serverless-operator/pull/415#discussion_r1519597852
##########
controllers/platform/services/properties.go:
##########
@@ -159,24 +159,25 @@ func generateReactiveURL(postgresSpec
*operatorapi.PersistencePostgreSQL, schema
// Never nil.
func GenerateDataIndexWorkflowProperties(workflow *operatorapi.SonataFlow,
platform *operatorapi.SonataFlowPlatform) (*properties.Properties, error) {
props := properties.NewProperties()
- props.Set(constants.KogitoProcessDefinitionsEventsEnabled, "false")
- props.Set(constants.KogitoProcessInstancesEventsEnabled, "false")
di := NewDataIndexHandler(platform)
- if workflow != nil && !profiles.IsDevProfile(workflow) &&
di.IsServiceEnabled() {
- props.Set(constants.KogitoProcessDefinitionsEventsEnabled,
"true")
-
props.Set(constants.KogitoProcessDefinitionsEventsErrorsEnabled, "true")
- props.Set(constants.KogitoProcessInstancesEventsEnabled, "true")
- props.Set(constants.KogitoDataIndexHealthCheckEnabled, "true")
- di := NewDataIndexHandler(platform)
- p, err := di.GenerateWorkflowProperties()
- if err != nil {
- return nil, err
+ if !profiles.IsDevProfile(workflow) && workflow != nil &&
workflow.Status.Services != nil && workflow.Status.Services.DataIndexRef != nil
{
+ serviceBaseUrl := workflow.Status.Services.DataIndexRef.Url
+ if di.IsServiceEnabled() && len(serviceBaseUrl) > 0 {
+
props.Set(constants.KogitoProcessDefinitionsEventsEnabled, "true")
+
props.Set(constants.KogitoProcessDefinitionsEventsErrorsEnabled, "true")
+
props.Set(constants.KogitoProcessInstancesEventsEnabled, "true")
+ props.Set(constants.KogitoDataIndexHealthCheckEnabled,
"true")
+ props.Set(constants.KogitoDataIndexURL, serviceBaseUrl)
+ props.Set(constants.KogitoProcessDefinitionsEventsURL,
serviceBaseUrl+constants.KogitoProcessDefinitionsEventsPath)
+ props.Set(constants.KogitoProcessInstancesEventsURL,
serviceBaseUrl+constants.KogitoProcessInstancesEventsPath)
}
- props.Merge(p)
+ } else {
+ props.Set(constants.KogitoProcessDefinitionsEventsEnabled,
"false")
Review Comment:
I'd suggest moving the setting of this two properties to the begin of the
processing as we did before.
```
props.Set(constants.KogitoProcessDefinitionsEventsEnabled, "false")
props.Set(constants.KogitoProcessInstancesEventsEnabled, "false")
```
Because, now, with this change, we have
if (frst level condition) {
if (second level condition) {
//setting to true if second level condition is true, but if not, we
still need the workflow to have them set to false.
props.Set(constants.KogitoProcessDefinitionsEventsEnabled,
"true")
props.Set(constants.KogitoProcessInstancesEventsEnabled, "true")
}
}
##########
controllers/platform/services/properties.go:
##########
@@ -185,19 +186,21 @@ func GenerateDataIndexWorkflowProperties(workflow
*operatorapi.SonataFlow, platf
// Never nil.
func GenerateJobServiceWorkflowProperties(workflow *operatorapi.SonataFlow,
platform *operatorapi.SonataFlowPlatform) (*properties.Properties, error) {
props := properties.NewProperties()
- props.Set(constants.JobServiceRequestEventsConnector,
constants.QuarkusHTTP)
Review Comment:
Same comment as for the the DataIndex, It's more safe to keep this setting
here.
##########
controllers/platform/services/properties.go:
##########
@@ -185,19 +186,21 @@ func GenerateDataIndexWorkflowProperties(workflow
*operatorapi.SonataFlow, platf
// Never nil.
func GenerateJobServiceWorkflowProperties(workflow *operatorapi.SonataFlow,
platform *operatorapi.SonataFlowPlatform) (*properties.Properties, error) {
props := properties.NewProperties()
- props.Set(constants.JobServiceRequestEventsConnector,
constants.QuarkusHTTP)
- props.Set(constants.JobServiceRequestEventsURL,
fmt.Sprintf("%s://localhost/v2/jobs/events", constants.JobServiceURLProtocol))
js := NewJobServiceHandler(platform)
- if workflow != nil && !profiles.IsDevProfile(workflow) &&
js.IsServiceEnabled() {
- if workflowdef.HasTimeouts(workflow) {
- props.Set(constants.KogitoJobServiceHealthCheckEnabled,
"true")
- }
- p, err := js.GenerateWorkflowProperties()
- if err != nil {
- return nil, err
+ if !profiles.IsDevProfile(workflow) && workflow != nil &&
workflow.Status.Services != nil && workflow.Status.Services.JobServiceRef !=
nil {
+ serviceBaseUrl := workflow.Status.Services.JobServiceRef.Url
Review Comment:
I see something that I believe more difficult to follow now, or potentially
error prone. Let me explain:
**Before:**
The jobs-service url was calculated by the JobServiceHandler, and there,
it was derived somehow directly from that service, or, from the cluster
platform if any.
**Now:**
Get get the jobs-service url from the
workflow.Status.Services.JobServiceRef, BUT, the workflow is normally what is
being deployed at this point, and that somehow "caused" the execution of all
this cycle. So we configure some WF properties from values from the WF itself.
I am correct?
Same reasoning apply for the DI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
