-
Notifications
You must be signed in to change notification settings - Fork 24
Ama cli docker #56
base: 0.3.0-dev
Are you sure you want to change the base?
Ama cli docker #56
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to the comments above you have 4 .zip files that shouldn't be there so please delete them?
Also what happened to ApplicationMaster.kt and YarnNMCallbackHandler.kt?
import org.apache.amaterasu.leader.common.launcher.ArgsParser | ||
import org.apache.amaterasu.leader.common.utilities.MessagingClientUtil | ||
|
||
class AppMasterArgsParser: ArgsParser() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is YARN specific, the CLI should have it's own parser
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is not here any more, isnt it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW i am using a libary argparser for Kotlin CLI
|
||
|
||
class ContainerHandler(val action : String) { | ||
val logger = LoggerFactory.getLogger(ContainerHandler::class.java) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can simply derive from KLogging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i know, i was just trying to make it work, at this phase, will switch to klogging
.build() | ||
} | ||
|
||
private fun getDockerClient(config: DefaultDockerClientConfig, dockerCmdExecFactory: JerseyDockerCmdExecFactory?): DockerClient { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't this three functions be a single expression funs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unfortunately not. we haven't introduced the TLS and cert layer as well. it will turn into a monster soon.
is this are the files ? ./sdk_python/dist/amaterasu-sdk-0.2.0-incubating-rc4.zip |
With the exception of those who ware deleted with the whole runner project: |
done. |
Adding support for BUILD docker image with AMA-CLI