zentol commented on a change in pull request #17474:
URL: https://github.com/apache/flink/pull/17474#discussion_r735351658



##########
File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/webmonitor/RestfulGateway.java
##########
@@ -132,37 +134,49 @@
             requestTaskManagerMetricQueryServiceAddresses(@RpcTimeout Time 
timeout);
 
     /**
-     * Triggers a savepoint with the given savepoint directory as a target.
+     * Triggers a savepoint with the given savepoint directory as a target, 
returning a future that
+     * completes when the operation is started.
      *
-     * @param jobId ID of the job for which the savepoint should be triggered.
-     * @param targetDirectory Target directory for the savepoint.
+     * @param operationKey the key of the operation, for deduplication purposes
+     * @param parameters input parameters for taking a savepoint
      * @param timeout Timeout for the asynchronous operation
-     * @return A future to the {@link CompletedCheckpoint#getExternalPointer() 
external pointer} of
-     *     the savepoint.
+     * @return Future which is completed once the operation is triggered 
successfully
      */
-    default CompletableFuture<String> triggerSavepoint(
-            JobID jobId, String targetDirectory, boolean cancelJob, 
@RpcTimeout Time timeout) {
+    default CompletableFuture<Acknowledge> triggerSavepoint(
+            AsynchronousJobOperationKey operationKey,
+            TriggerSavepointParameters parameters,

Review comment:
       If you had replaced the `AsynchronousJobOperationKey` with a plain 
`TriggerId` then you could've kept the `JobID` in the 
`TriggerSavepointParameters` and have the `triggerSavepoint*` method signatures 
be nearly identical; with the only difference being the presence of the 
`triggerId.`




-- 
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]


Reply via email to