[NU-1806] Add action parameters #6860

gskrobisz · 2024-09-13T11:39:15Z

Describe your changes

Source defines parameters that are required by action (here action deploy). When scenario has multiple sources each source defines those parameters separately.
FE fetches those parameters and displays "advanced parameters" form in deploy action window (see screenshot above).
Values are provided as simple kv map to deploy endpoint. So far deploy required plain text comment as body, now it requires json with comment and nodes deployment data (optional).
Nodes deployment data are passed further into deployment command, and can be used to initialize flink source function.

Checklist before merge

Related issue ID is placed at the beginning of PR title in [brackets] (can be GH issue or Nu Jira issue)
Code is cleaned from temporary changes and commented out lines
Parts of the code that are not easy to understand are documented in the code
Changes are covered by automated tests
Showcase in dev-application.conf added to demonstrate the feature
Documentation added or updated
Added entry in Changelog.md describing the change from the perspective of a public distribution user
Added MigrationGuide.md entry in the appropriate subcategory if introducing a breaking change
Verify that PR will be squashed during merge

github-actions · 2024-09-13T12:12:05Z

created: #7461
⚠️ Be careful! Snapshot changes are not necessarily the cause of the error. Check the logs.

common-api/src/main/scala/pl/touk/nussknacker/engine/api/definition/FixedExpressionValue.scala

...onents-api/src/main/scala/pl/touk/nussknacker/engine/api/component/NodesDeploymentData.scala

components-api/src/main/scala/pl/touk/nussknacker/engine/api/definition/ParameterEditor.scala

components-api/src/main/scala/pl/touk/nussknacker/engine/api/process/Source.scala

mgoworko · 2024-12-20T11:03:15Z

...r/server/src/main/scala/pl/touk/nussknacker/ui/process/newactivity/ActivityInfoService.scala

+import pl.touk.nussknacker.ui.uiresolving.UIProcessResolver
+
+// TODO: move to ActivityService? execute node compilation only once with ScenarioTestService?
+class ActivityInfoService(activityInfoProvider: ActivityInfoProvider, processResolver: UIProcessResolver) {


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.

Renamed: activity -> action

"activity" is still used in some contexts, such as nodeToActivityToParameters, FE const activityParameters, const activityNodeParameters and possibly some more usages.

designer/server/src/main/scala/pl/touk/nussknacker/ui/api/ActivityInfoResources.scala

add forceLatestRead to dev-application.conf fixed radiobutton options

Dzuming

I left some comments

Dzuming · 2025-01-21T11:01:48Z

designer/client/src/components/graph/node-modal/editors/expression/FixedValuesEditor.tsx

@@ -46,6 +51,7 @@ export const FixedValuesEditor: ExtendedEditor<Props> = (props: Props) => {
    };

    const { expressionObj, readOnly, onValueChange, className, showValidation, editorConfig, fieldErrors } = props;
+    const mode = FixedValuesEditorMode[editorConfig.mode || "LIST"];


What's the reason for introducing editor mode concept instead of adding a new editor

Dzuming · 2025-01-21T11:03:33Z

designer/client/src/components/toolbars/scenarioActions/buttons/DeployButton.tsx

@@ -23,6 +25,9 @@ export default function DeployButton(props: ToolbarButtonProps) {
    const capabilities = useSelector(getCapabilities);
    const { disabled, type } = props;

+    // TODO: find better place to reload activity capabilities and properties


What about this todo, will you resolve it in this pr?

Dzuming · 2025-01-21T11:06:36Z

designer/client/src/components/modals/DeployWithParametersDialog.tsx

+                        Advanced parameters
+                    </Typography>
+                ) : null}
+                {activityNodeParameters.map((anp: ActionNodeParameters) => (


This acronym was a little confusing to me. I needed to go back to activityNodeParameters to understand what was going on. WDYT about renaming it to activityNodeParameter?

Dzuming · 2025-01-21T11:06:47Z

designer/client/src/components/modals/DeployWithParametersDialog.tsx

+                            textDecoration: "none",
+                        })}
+                    >
+                        Advanced parameters


Missing i18n

Dzuming · 2025-01-21T11:07:09Z

designer/client/src/components/modals/ActionParameter.tsx

+import { default as EditableEditor } from "../graph/node-modal/editors/EditableEditor";
+
+interface Props {
+    nodeName: string;


should it be nodeName or nodeId?

Dzuming · 2025-01-21T11:14:28Z

designer/client/src/components/modals/DeployWithParametersDialog.tsx

+        } catch (error) {
+            setValidationError(error?.response?.data);
+        }
+    }, [action, comment, dispatch, processName, props]);


Missing values dependency here

Dzuming · 2025-01-21T11:17:05Z

designer/client/src/components/modals/GenericAction/useActionCapabilities.tsx

+import { getProcessName, getScenarioGraph } from "../../../reducers/selectors/graph";
+import { useEffect } from "react";
+import { fetchActionParameters } from "../../../actions/nk";
+export function useActionCapabilities() {


Can you add blank line before import?

Dzuming · 2025-01-21T11:18:00Z

designer/client/src/components/toolbars/scenarioActions/buttons/DeployButton.tsx

@@ -39,7 +44,7 @@ export default function DeployButton(props: ToolbarButtonProps) {
    const { open } = useWindows();

    const message = t("panels.actions.deploy.dialog", "Deploy scenario {{name}}", { name: processName });
-    const action = (p, c) => HttpService.deploy(p, c).finally(() => dispatch(loadProcessState(processName, processVersionId)));
+    const action = (p, c, d) => HttpService.deploy(p, c, d).finally(() => dispatch(loadProcessState(processName, processVersionId)));


Can we find a better name for these (p, c, d) parameters?

Dzuming · 2025-01-21T11:18:14Z

designer/client/src/components/toolbars/scenarioActions/buttons/RunOffScheduleButton.tsx

@@ -34,7 +34,7 @@ export default function RunOffScheduleButton(props: ToolbarButtonProps) {
    const available = !disabled && isPossible && capabilities.deploy;

    const { open } = useWindows();
-    const action = (p, c) => HttpService.runOffSchedule(p, c).finally(() => dispatch(loadProcessState(processName, processVersionId)));
+    const action = (p, c, d) => HttpService.runOffSchedule(p, c).finally(() => dispatch(loadProcessState(processName, processVersionId)));


Can we find a better name for these (p, c, d) parameters?

Dzuming · 2025-01-21T11:27:07Z

designer/client/src/http/HttpService.ts

    ): Promise<{
        isSuccess: boolean;
    }> {
+        const runDeploymentRequest = {


Can't we just simplify it to

const runDeploymentRequest = { nodesDeploymentData, comment };
?

Undefined values will be removed automatically from a JSON.

Alternatively, if we want to do it explicitly, maybe can we use lodash?

const runDeploymentRequest = _.pickBy({comment, nodesDeploymentData});

github-actions bot mentioned this pull request Sep 13, 2024

Update Cypress snapshots in activity_capabilities #6861

Closed

gskrobisz force-pushed the activity_capabilities branch 3 times, most recently from be0fe74 to e0b7858 Compare September 25, 2024 22:37

github-actions bot added client client main fe ui labels Sep 25, 2024

gskrobisz force-pushed the activity_capabilities branch from 8dd895f to 68932ac Compare October 1, 2024 12:36

github-actions bot mentioned this pull request Oct 1, 2024

Update Cypress snapshots in activity_capabilities #6968

Closed

gskrobisz force-pushed the activity_capabilities branch 2 times, most recently from 214c008 to 6eb498d Compare December 17, 2024 22:56

mgoworko reviewed Dec 20, 2024

View reviewed changes

gskrobisz force-pushed the activity_capabilities branch from 6eb498d to 62b2354 Compare December 20, 2024 18:28

gskrobisz changed the title ~~[NU-1806] Add activity parameters~~ [NU-1806] Add action parameters Jan 7, 2025

gskrobisz force-pushed the activity_capabilities branch from 2413cc2 to f763629 Compare January 9, 2025 16:55

github-actions bot mentioned this pull request Jan 13, 2025

Update Cypress snapshots in activity_capabilities #7429

Merged

gskrobisz marked this pull request as ready for review January 14, 2025 13:06

gskrobisz force-pushed the activity_capabilities branch from bfe6011 to f0db0a0 Compare January 15, 2025 10:30

github-actions bot added the docs label Jan 15, 2025

gskrobisz force-pushed the activity_capabilities branch from 8872b48 to 43a81b6 Compare January 15, 2025 18:23

github-actions bot mentioned this pull request Jan 15, 2025

Update Cypress snapshots in activity_capabilities #7461

Closed

gskrobisz added 9 commits January 20, 2025 15:23

[NU-1806] Add deployment activity parameters

6599aa0

revert hintText

3bf58f0

Use offset strategy in kafkasource

ac45a3a

Use offset strategy in FlinkKafkaSource

d06881a

revert mui

c9aee77

separate modal

92c8831

PR review: rename activity->action

451bb6b

PR review: rename activity->action

b08dc2b

add label advanced parameters

5965e84

add forceLatestRead to dev-application.conf fixed radiobutton options

gskrobisz and others added 5 commits January 20, 2025 15:23

move WithActionParametersSupport

4352414

revert config.run.xml

311f5b9

Fix CI

241058a

Updated snapshots (#7429)

7ecefa2

Tapir endpoint

cf9611b

gskrobisz force-pushed the activity_capabilities branch from cf58d86 to cf9611b Compare January 20, 2025 14:25

Dzuming self-requested a review January 21, 2025 10:55

Dzuming requested changes Jan 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NU-1806] Add action parameters #6860

[NU-1806] Add action parameters #6860

gskrobisz commented Sep 13, 2024 •

edited

Loading

github-actions bot commented Sep 13, 2024 •

edited

Loading

mgoworko Dec 20, 2024 •

edited

Loading

gskrobisz Jan 9, 2025

mgoworko Jan 21, 2025

Dzuming left a comment

Dzuming Jan 21, 2025

Dzuming Jan 21, 2025

Dzuming Jan 21, 2025

Dzuming Jan 21, 2025

Dzuming Jan 21, 2025

Dzuming Jan 21, 2025

Dzuming Jan 21, 2025

Dzuming Jan 21, 2025

Dzuming Jan 21, 2025

Dzuming Jan 21, 2025

[NU-1806] Add action parameters #6860

Are you sure you want to change the base?

[NU-1806] Add action parameters #6860

Conversation

gskrobisz commented Sep 13, 2024 • edited Loading

Describe your changes

Checklist before merge

github-actions bot commented Sep 13, 2024 • edited Loading

mgoworko Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Dzuming left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gskrobisz commented Sep 13, 2024 •

edited

Loading

github-actions bot commented Sep 13, 2024 •

edited

Loading

mgoworko Dec 20, 2024 •

edited

Loading