wmedvede commented on code in PR #443:
URL:
https://github.com/apache/incubator-kie-kogito-serverless-operator/pull/443#discussion_r1567101577
##########
controllers/profiles/common/properties/managed_test.go:
##########
@@ -121,11 +121,11 @@ func
Test_appPropertyHandler_WithUserPropertiesWithNoUserOverrides(t *testing.T)
//just add some user provided properties, no overrides.
userProperties := "property1=value1\nproperty2=value2"
workflow := test.GetBaseSonataFlow("default")
- props, err := NewAppPropertyHandler(workflow, nil)
+ props, err := NewManagedPropertyHandler(workflow, nil)
assert.NoError(t, err)
generatedProps, propsErr :=
properties.LoadString(props.WithUserProperties(userProperties).Build())
assert.NoError(t, propsErr)
- assert.Equal(t, 6, len(generatedProps.Keys()))
+ assert.Equal(t, 7, len(generatedProps.Keys()))
Review Comment:
sorry for the silly question, but why are we now expecting 7 properties
instead of 6?
a new by default property was introduced?
##########
controllers/profiles/common/properties/managed_test.go:
##########
@@ -149,15 +149,15 @@ func
Test_appPropertyHandler_WithUserPropertiesWithServiceDiscovery(t *testing.T
userProperties = userProperties +
"broker2=${knative:brokers.v1.eventing.knative.dev/my-kn-broker2}\n"
workflow := test.GetBaseSonataFlow(defaultNamespace)
- props, err := NewAppPropertyHandler(workflow, nil)
+ props, err := NewManagedPropertyHandler(workflow, nil)
assert.NoError(t, err)
generatedProps, propsErr := properties.LoadString(props.
WithUserProperties(userProperties).
WithServiceDiscovery(context.TODO(), &mockCatalogService{}).
Build())
generatedProps.DisableExpansion = true
assert.NoError(t, propsErr)
- assert.Equal(t, 20, len(generatedProps.Keys()))
+ assert.Equal(t, 21, len(generatedProps.Keys()))
Review Comment:
same here
##########
controllers/profiles/common/properties/managed.go:
##########
@@ -157,28 +158,34 @@ func NewAppPropertyHandler(workflow
*operatorapi.SonataFlow, platform *operatora
return nil, err
}
props.Merge(p)
- p, err = generateKnativeEventingWorkflowProperties(workflow)
+ p, err = resolvePlatformWorkflowProperties(platform)
Review Comment:
So this properties might eventually overwrite the properties configured in
the previous lines for the workflow to connect with the DI and JS?
--
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]
