ricardozanini commented on code in PR #372:
URL:
https://github.com/apache/incubator-kie-kogito-serverless-operator/pull/372#discussion_r1480322234
##########
test/testdata/workflow/persistence/by_service/02-sonataflow_platform.yaml:
##########
@@ -21,7 +21,7 @@ spec:
template:
buildArgs:
- name: QUARKUS_EXTENSIONS
- value:
org.kie.kogito:kogito-addons-quarkus-jobs-knative-eventing:999-SNAPSHOT,org.kie.kogito:kogito-addons-quarkus-persistence-jdbc:999-SNAPSHOT,io.quarkus:quarkus-jdbc-postgresql:3.2.9.Final,io.quarkus:quarkus-agroal:3.2.9.Final
+ value:
org.kie.kogito:kogito-addons-quarkus-persistence-jdbc:999-SNAPSHOT,io.quarkus:quarkus-jdbc-postgresql:3.2.9.Final,io.quarkus:quarkus-agroal:3.2.9.Final
Review Comment:
I guess we can add these values by default when persistence is on.
@jordigilh can you please open a follow-up issue? I'll do an assessment.
##########
controllers/profiles/common/object_creators.go:
##########
@@ -157,7 +159,13 @@ func defaultContainer(workflow *operatorapi.SonataFlow)
(*corev1.Container, erro
if err := mergo.Merge(defaultFlowContainer,
workflow.Spec.PodTemplate.Container.ToContainer(), mergo.WithOverride); err !=
nil {
return nil, err
}
- defaultFlowContainer = ConfigurePersistence(defaultFlowContainer,
workflow.Spec.Persistence, defaultSchemaName, workflow.Namespace)
+ if workflow.Spec.Persistence != nil {
Review Comment:
It makes sense. But what about adding a label to address this problem
`sonataflow.org/persistence=platform|workflow|none`, defaults to none.
The problem with using this approach, @jordigilh @tchughesiv is that the API
is not communicating what will be done. It's an error prune having
`.spec.persistence: {}` meaning that we will pull from the platform.
On the other hand, having `.spec.persistence: <data>` clears communicate
that we are overriding. But `.spec.persistence: {}` or not having this
attribute defined is dubious IMO. One can interpret that it will pull from
platform if the attribute is not there.
##########
controllers/profiles/dev/states_dev.go:
##########
@@ -70,15 +73,14 @@ func (e *ensureRunningWorkflowState) Do(ctx
context.Context, workflow *operatora
devBaseContainerImage := workflowdef.GetDefaultWorkflowDevModeImageTag()
// check if the Platform available
- pl, err := platform.GetActivePlatform(ctx, e.C, workflow.Namespace)
- if err == nil && len(pl.Spec.DevMode.BaseImage) > 0 {
- devBaseContainerImage = pl.Spec.DevMode.BaseImage
+ if plf != nil && len(plf.Spec.DevMode.BaseImage) > 0 {
+ devBaseContainerImage = plf.Spec.DevMode.BaseImage
}
userPropsCM, _, err := e.ensurers.userPropsConfigMap.Ensure(ctx,
workflow)
if err != nil {
return ctrl.Result{Requeue: false}, objs, err
}
- managedPropsCM, _, err := e.ensurers.managedPropsConfigMap.Ensure(ctx,
workflow, pl, common.ManagedPropertiesMutateVisitor(ctx,
e.StateSupport.Catalog, workflow, pl, userPropsCM.(*corev1.ConfigMap)))
+ managedPropsCM, _, err := e.ensurers.managedPropsConfigMap.Ensure(ctx,
workflow, plf, common.ManagedPropertiesMutateVisitor(ctx,
e.StateSupport.Catalog, workflow, plf, userPropsCM.(*corev1.ConfigMap)))
Review Comment:
Why have you changed to `pl` in some cases and here you use `plf`? 😄
--
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]
