[NU-1806] Add activity parameters #6860
Conversation
|
updated: #6968 |
gskrobisz
force-pushed
the
activity_capabilities
branch
3 times, most recently
from
be0fe74 to
e0b7858
Compare
gskrobisz
force-pushed
the
activity_capabilities
branch
from
8dd895f to
68932ac
Compare
gskrobisz
force-pushed
the
activity_capabilities
branch
2 times, most recently
from
214c008 to
6eb498d
Compare
|
|
||
| object FixedExpressionValue { | ||
| val nullFixedValue: FixedExpressionValue = FixedExpressionValue("", "") | ||
| def apply(expression: String, label: String): FixedExpressionValue = FixedExpressionValue(expression, label, None) |
There was a problem hiding this comment.
Is this apply method required? In code itself it is not required, because there is default value for hintText, but maybe I'm missing sth related to some integrations/Flink/Java.
| // Raw deployment parameters (name -> value) that are used as additional node configuration during deployment. | ||
| // Each node can be provided with dedicated set of parameters. | ||
| // TODO: consider replacing NodeDeploymentData with Json | ||
| type NodeDeploymentData = Map[String, String] |
There was a problem hiding this comment.
I think it should be a final case class, serializable to JSON. Now it is a map, serializable to Json object. So adding any new field beside the map of raw params would break compatibility
| implicit val fixedValuesParameterEditorEncoder: Encoder[FixedValuesParameterEditor] = | ||
| deriveEncoder[FixedValuesParameterEditor] | ||
|
|
||
| implicit val fixedValuesParameterEditorDecoder: Decoder[FixedValuesParameterEditor] = { (c: HCursor) => |
There was a problem hiding this comment.
suggestion:
implicit val fixedValuesParameterEditorDecoder: Decoder[FixedValuesParameterEditor] =
Decoder.forProduct2("possibleValues", "mode")(FixedValuesParameterEditor.apply)
.or(Decoder.forProduct1("possibleValues")(FixedValuesParameterEditor(_, FixedValuesEditorMode.LIST)))
| * {"DEPLOY": { "parametername": ...parameter configuration... } | ||
| */ | ||
| trait WithActivityParameters { self: Source => | ||
| def activityParametersDefinition: Map[String, Map[String, ParameterConfig]] |
There was a problem hiding this comment.
Maybe it would be a good idea to define enum (sealed trait) with all possible scenario actions in the API module and make it the key of this map. Right now if we change the name of some scenario action, the change will not be enforceable by compiler in all implementations of this trait.
| import pl.touk.nussknacker.ui.uiresolving.UIProcessResolver | ||
|
|
||
| // TODO: move to ActivityService? execute node compilation only once with ScenarioTestService? | ||
| class ActivityInfoService(activityInfoProvider: ActivityInfoProvider, processResolver: UIProcessResolver) { |
There was a problem hiding this comment.
Comment about naming convention: I think that in this context the name "Scenario action" should be used. Deploy/cancel and others are scenario actions, not activities from what I know. It can be especially confusing now, when ScenarioActivities exist in different context.
| with ProcessDirectives | ||
| with LazyLogging { | ||
|
|
||
| def securedRoute(implicit user: LoggedUser): Route = { |
There was a problem hiding this comment.
We discussed returning this information in the already existing endpoint /process/{name}/"status". I think that it would be a good idea, but I'm not sure if it would be easy or doable without making some POC. If not possible, then I think the new service should be a Tapir service, with "ActionInfo" in name instead of "ActivityInfo"
gskrobisz
force-pushed
the
activity_capabilities
branch
from
6eb498d to
62b2354
Compare
Describe your changes
Checklist before merge