Change AuroraController to an interface #1672
Conversation
| auroraCmd.add("--batch-size"); | ||
| auroraCmd.add(Integer.toString(Integer.MAX_VALUE)); | ||
| } | ||
| boolean createJob(String auroraFilename, Map<String, String> bindings); |
There was a problem hiding this comment.
Should we add some documentation here? What is bindings here? I need to read aurora job create -h to know what it is. Considering this is an argument of an interface function, I think it is necessary to add some docs here.
There was a problem hiding this comment.
Good catch. I refactored createJob to remove the auroraFilename, which only applies to the CLI impl. I also renamed bindings to auroraProperties and changed the key from a string to an enum to make it's usage clear.
@objmagic I was in the process of making add'l edits when you gave your ship it. Would you please take a quick look again. I'm done making changes.
| boolean restartJob(int containerId); | ||
|
|
||
| /** | ||
| * Restarts a given container, or the entire job if containerId is null |
There was a problem hiding this comment.
when containerId is -1, the entire job will also be restarted according to here
There was a problem hiding this comment.
Although not the preferred approach I kept that for backward compatibility, but there's no need to do that since the code will all change in lock step. I've made the change to match behavior to docs.
|
LGTM. |
* Change AuroraController to an interface
The current implementation of
AuroraControllershells out to the aurora cli and is intended to be run locally by a developer. Other implementations might call the aurora scheduler service directly. ChangingAuroraControllerto be an interface allows other implementations to be used byAuroraScheduler.The diff makes things appear messier than they are but basically
AuroraControllerwas renamed toAuroraCLIControllerand a newAuroraControllerinterface was added.