[GitHub] [arrow-ballista] thinkharderdev commented on pull request #560: Cluster state refactor part 1

[GitBox <gi...@apache.org>]

thinkharderdev commented on PR #560:
URL: https://github.com/apache/arrow-ballista/pull/560#issuecomment-1398473523

   > I'm not as familiar with the overall Ballista architecture as I'd like to be, but this PR still makes a lot of sense.
   > 
   > I _love_ seeing this stuff come out of the `executor_manager` which seemed like it didn't have a clear boundary on what it's role was and go into a more self-contained `cluster.rs`.
   > 
   > I like that it encapsulates all the HA session management stuff, because I think we'll need options like Redis and others in different enterprise environments.
   > 
   > The only thing that makes me sad is seeing 2 values (state + backend) get passed around everywhere instead of 1. That's really not the fault of this PR however, as mentioned in my other comment, I think we need to clean the plumbing up around config in general.
   
   Yeah, I agree. I'm workring on the second part of this PR (which will implement the `JobState` interface) and will take a crack at cleaning this up. 


-- 
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-unsubscribe@arrow.apache.org

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