-
Notifications
You must be signed in to change notification settings - Fork 66
Conversation
def validate(self) -> bool: | ||
""" | ||
Validate the config at its current state. | ||
Raises: Error if invalid | ||
""" | ||
if self.toolkit is None: | ||
raise error.InvalidModelError( | ||
"Please supply a toolkit for the cluster") |
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.
Maybe we should list available toolkits?
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.
aztk/models/toolkit.py
Outdated
|
||
TOOLKIT_MAP = dict( | ||
spark=ToolkitDefinition( | ||
versions=["1.6", "2.1", "2.2"], |
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.
do we care about minor revisions?
Not sure if we support it, but 1.6 is actually 1.6.3. Also, 2.2 had several revisions to it as well, so probably worth being explicit.
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 depends on how the docker images are created. I feel like we should not care about this here.
We are not going to provide multiple docker images per minor version. So being more precise will just break people when we update docker image 2.2.3 -> 2.2.4 for example
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 guess we need to answer that question then. Are we going to do a 2.2.3 -> 2.2.4? I guess users probably shouldn't care too much about that and if they do, they can use a custom docker image... Seems fine then.
aztk/models/toolkit.py
Outdated
self._validate_required(["software", "version"]) | ||
|
||
if self.software not in TOOLKIT_MAP: | ||
raise InvalidModelError("Toolkit {0} is not in the list of allowed toolkits {1}".format( |
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.
nitpick - add ' around the toolkit name
Toolkit 'spark' is not ...
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.
Same for print statements below
fix #503 Added a new toolkit key to abstract away the complexity of docker repo versioning.
Include error throwing if using invalid images which is a step in the direction for #266
Browse available toolkits with the
aztk toolkit
command