New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Remove MVAD's dependence on hardwired credentials and azure SDKs #1629
Conversation
Hey @mhamilton723 👋! |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Codecov Report
@@ Coverage Diff @@
## master #1629 +/- ##
=======================================
Coverage 82.81% 82.81%
=======================================
Files 286 286
Lines 15064 15064
Branches 745 745
=======================================
Hits 12476 12476
Misses 2588 2588 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
3c908f2
to
9cd485c
Compare
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Thanks for all those crazy refactor!! It's super clean now and easier to maintain in the future 😄 Hooray!
val intermediateSaveDir = new Param[String](this, "intermediateSaveDir", "Directory name " + | ||
"of which you want to save the intermediate data produced while training.") | ||
private def validateIntermediateSaveDir(dir: String): Boolean = { | ||
assert(dir.startsWith("wasbs://"), "improper HDFS loacation. Please use a wasb path such as: \n" + |
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.
Is only wasbs accepted? What about abfss and say in Trident some other protocols?
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.
Added abfss too
private def getStorageInfo: StorageInfo = { | ||
val uri = new URI(getIntermediateSaveDir) | ||
val account = uri.getHost.split(".".toCharArray).head | ||
val config = s"fs.azure.account.key.$account.blob.core.windows.net" |
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.
Does this config also work on databricks?
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.
yes!
val errors = summary.errors.get.toJson.compactPrint | ||
throw new RuntimeException(s"Caught errors during inference: $errors") | ||
throw new RuntimeException(s"Could not get trained model: $errors") |
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 not can't get trained model
error I guess, it should be inference failure?
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.
Fixed
@@ -397,6 +348,19 @@ trait MADBase extends HasOutputCol | |||
|""".stripMargin | |||
} | |||
|
|||
protected def submitDataset(dataset: Dataset[_]): Map[String, JsValue] = { |
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.
Can we rename this method as it does both submitDataset
and send request to MVAD
?
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.
Fixed
9cd485c
to
20ea5e0
Compare
20ea5e0
to
58ca0e1
Compare
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Related Issues/PRs
#xxx
What changes are proposed in this pull request?
Briefly describe the changes included in this Pull Request.
How is this patch tested?
Does this PR change any dependencies?
Does this PR add a new feature? If so, have you added samples on website?
website/docs/documentation
folder.Make sure you choose the correct class
estimators/transformers
and namespace.DocTable
points to correct API link.yarn run start
to make sure the website renders correctly.<!--pytest-codeblocks:cont-->
before each python code blocks to enable auto-tests for python samples.WebsiteSamplesTests
job pass in the pipeline.AB#1944709