KR-bluejay commented on code in PR #1314:
URL: 
https://github.com/apache/datafusion-ballista/pull/1314#discussion_r2332563931


##########
ballista/executor/src/execution_loop.rs:
##########
@@ -88,8 +90,29 @@ pub async fn poll_loop<T: 'static + AsLogicalPlan, U: 
'static + AsExecutionPlan>
 
         match poll_work_result {
             Ok(result) => {
-                let tasks = result.into_inner().tasks;
+                let PollWorkResult {
+                    tasks,
+                    jobs_to_clean,
+                } = result.into_inner();
                 active_job = !tasks.is_empty();
+                let work_dir = PathBuf::from(&executor.work_dir);
+
+                // Clean up any state related to the listed jobs

Review Comment:
   ### 1. Thoughts on the existing function
   I looked at the `remove_job_data` function when writing this.  
   At that time, I considered splitting it out. Eventually, I decided it would 
be better to leave that refactoring for another issue and PR, because 
`remove_job_data` is a Protobuf response handler.
   
   ### 2. About unification
   Yeah, I could work on unifying the two job data removal functions.  
   Would it be okay if I take on this task?



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to