tchughesiv commented on code in PR #415:
URL:
https://github.com/apache/incubator-kie-kogito-serverless-operator/pull/415#discussion_r1520073718
##########
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:
no, we are still using the service handlers to set the urls. There's a new
method called `SetServiceUrlsInWorkflowStatus` that handles this in the
workflow(s) status -
https://github.com/apache/incubator-kie-kogito-serverless-operator/blob/2325390963e643afd9868174e9818d7dafde4317/controllers/platform/services/services.go#L432-L439
what HAS changed is... during a workflow's configmap properties creation,
instead of calling the handler directly, we're using the service url directly
from the workflow's status... which has already been properly set by the
service handlers.
this is actually LESS error prone because it ensures the accuracy of some
things, which, in `main` today can be a bit nebulous -
1. this ensures that a workflow's application properties match the correct
service urls which are set the status of a workflow. currently a user would
have to figure this out for themselves by either checking the config map, or
the platform object.
2. this ensures that application properties config maps are properly updated
in real-time when service changes are made in the platform/clusterplatform...
which, in turn, forces the workflow pod to restart and these new settings to
take affect. today, a user may have to remove the existing configmap to force
proper properties... which is problematic because they may not know this is
required and a workflow may be attempting to send to a non-existent (or
incorrect) service.
--
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]
