AWS S3 support #433
AWS S3 support #433
Conversation
| val protocol = getProtocol(marSavePath) | ||
|
|
||
| if ("file".equalsIgnoreCase(protocol)) { | ||
| print("Local") |
There was a problem hiding this comment.
remove or enhance these prints (or log)
There was a problem hiding this comment.
removed the debug statements
| jarFileList = jarFileList ::: List(new File(modelSrcDir.toString)) | ||
| } | ||
| else { | ||
| print("not local") |
| * @return The protocol for the given source. | ||
| */ | ||
| def getProtocol(source: String): String = { | ||
| require(source != null, "marfile source must not be null") |
| def getProtocol(source: String): String = { | ||
| require(source != null, "marfile source must not be null") | ||
|
|
||
| var protocol: String = null |
There was a problem hiding this comment.
make protocol a val and assign to the try statement directly
| catch { | ||
| case ex: Exception => | ||
| if (source.startsWith("//")) { | ||
| throw new IllegalArgumentException("Relative context: " + source) |
There was a problem hiding this comment.
"Relative context" - not sure how that message helps?
There was a problem hiding this comment.
changed it to: "Does not support Relative context: " + source
| result = file.toURI.toURL.getProtocol | ||
| } | ||
| catch { | ||
| case ex: Exception => result = "unknown" |
There was a problem hiding this comment.
I'm not a fan of this catch and return "unknown" --this mean you've go to have logic somewhere else to make sense of it. It's stronger to let the exception run free or at least have this method return a Try object.
| val protocol = ScoringModelUtils.getProtocol(storagePath) | ||
|
|
||
| if ("file".equalsIgnoreCase(protocol)) { | ||
| print("Local") |
There was a problem hiding this comment.
improve print message.
Does this really mean local? What about s3? --does it fall under local or hdsf? Could provide more help in making the right set of options, besides local and hdfs?
Also, this reveals to me that the protocol/path work you've added above is not unique to ScoringModelUtils, but more generally to sparktk saveload. That logic should move to this more generic location. What do you think?
No description provided.