fix: Make SynapseE2E Tests work now with Spark 3.2 #1362

riserrad · 2022-01-25T05:09:29Z

Problem

SynapseE2E tests likely stopped passing after two events:

it started building & testing SynapseML 0.9.5, given it only supports Spark 3.2.
- The reason is that the Synapse Analytics Workspace it was using to run the Jobs only had Apache Spark Pools using Spark 3.1.
the notebooks it was using to submit the jobs were moved from the /notebooks folder to subfolders in /notebooks/features, and the current code did not support fetching files from folders recursively.

Changes

Given these issues, the following changes are presented in this PR:

FileUtilities.scala now presents a function to read files in a folder recursively. This function is later consumed by SynapseUtilities.scala;
In SynapseTests.scala:
- Made the tests now submit jobs to the workspace that supports Spark 3.2 (private preview);
- To make sure the tests succeed and run in a timely fashion, it is now using 5 pools instead of 3.
  - Currently, since it has 13 Jobs, it requires 3 batches to complete. If we want this to run even faster, we could add 2 more pools and make it run in 2 baches only.
In SynapseUtilities.scala:
- Fixed the livyPayload to install synapseml_2.12 instead of synapseml;
- Fixed the livyPayload to also exclude org.slf4j:slf4j-api when configuring the session;
- Increased the test timeout from 20 to 30 minutes - made this change by observing the average time jobs were taking and 20 minutes seemed not to be enough. Empirically, 30 minutes worked well.

Additionally, as I was going through this, I noticed some opportunities for improvement in developer-readme:

Added some links to additional software it needs, and for a newcomer it might not be obvious to have installed (e.g., JDK 11, Miniconda);
Added additional links to guidance that might be useful for folks getting started, such as "Forking a repo" and "Working with remotes";
Added an additional step to prepare the Python/Conda environment
- This one might be needed to run tests manually, run nbconvert and all that.

Results

After these changes were pushed to the fork, we were able to see consistent success in the Pipeline execution.

Opportunities

There are still opportunties for improvements here, but I'd rather have them in a separate PR/effort, such as:

Understand the dependency on Azure CLI when setting up the local development environment.
- Some folks went through issues when setting it up, when not having the cli installed/configured.
Have a separate test result for each notebook run in SynapseE2E
- Today, if any job fails in SynapseE2E, it's hard to identify which job(s) failed for troubleshooting.
- We can leverage Scala's runtime test generation for that.
Manage the Desired State Configuration of the SynapseE2E Workspace in code
- It is risky not to store the Desired State Configuration of this workspace our SynapseE2E tests use to run the tests, since if anyone makes a change to it (delete it, deletes a pool, etc), tests are going to break.

riserrad · 2022-01-25T05:10:24Z

/azp run

azure-pipelines · 2022-01-25T05:10:29Z

Commenter does not have sufficient privileges for PR 1362 in repo microsoft/SynapseML

serena-ruan · 2022-01-25T05:15:16Z

/azp run

azure-pipelines · 2022-01-25T05:15:26Z

Azure Pipelines successfully started running 1 pipeline(s).

codecov-commenter · 2022-01-25T05:21:32Z

Codecov Report

Merging #1362 (ba2cc60) into master (865d189) will decrease coverage by 0.10%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1362      +/-   ##
==========================================
- Coverage   84.85%   84.75%   -0.11%     
==========================================
  Files         287      287              
  Lines       14234    14239       +5     
  Branches      728      728              
==========================================
- Hits        12078    12068      -10     
- Misses       2156     2171      +15

Impacted Files	Coverage Δ
...soft/azure/synapse/ml/core/env/FileUtilities.scala	`63.63% <0.00%> (-11.37%)`	⬇️
...ala/org/apache/spark/ml/param/DataFrameParam.scala	`70.83% <0.00%> (-16.67%)`	⬇️
...crosoft/azure/synapse/ml/io/http/HTTPClients.scala	`76.66% <0.00%> (-13.34%)`	⬇️
.../execution/streaming/continuous/HTTPSourceV2.scala	`92.80% <0.00%> (+0.71%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 865d189...ba2cc60. Read the comment docs.

serena-ruan · 2022-01-25T06:24:31Z

/azp run

azure-pipelines · 2022-01-25T06:24:41Z

Azure Pipelines successfully started running 1 pipeline(s).

riserrad · 2022-01-27T19:13:19Z

/azp run

azure-pipelines · 2022-01-27T19:13:25Z

Commenter does not have sufficient privileges for PR 1362 in repo microsoft/SynapseML

svotaw · 2022-01-27T23:19:46Z

@svotaw is added to the review. #Closed

mhamilton723 · 2022-01-28T00:41:08Z

/azp run

azure-pipelines · 2022-01-28T00:41:18Z

Azure Pipelines successfully started running 1 pipeline(s).

…/riserrad/SynapseML into riserrad/make-synapse-tests-work

riserrad · 2022-01-28T01:36:07Z

/azp run

azure-pipelines · 2022-01-28T01:36:18Z

Azure Pipelines successfully started running 1 pipeline(s).

riserrad · 2022-01-28T02:53:14Z

/azp run

azure-pipelines · 2022-01-28T02:53:24Z

Azure Pipelines successfully started running 1 pipeline(s).

riserrad · 2022-01-28T08:31:08Z

/azp run

azure-pipelines · 2022-01-28T08:31:18Z

Azure Pipelines successfully started running 1 pipeline(s).

serena-ruan · 2022-01-28T10:44:39Z

/azp run

azure-pipelines · 2022-01-28T10:44:51Z

Azure Pipelines successfully started running 1 pipeline(s).

riserrad · 2022-01-28T15:46:00Z

/azp run

azure-pipelines · 2022-01-28T15:46:11Z

Azure Pipelines successfully started running 1 pipeline(s).

riserrad · 2022-01-28T18:19:38Z

/azp run

azure-pipelines · 2022-01-28T18:19:48Z

Azure Pipelines successfully started running 1 pipeline(s).

mhamilton723

Fantastique! Just minor comments

mhamilton723 · 2022-01-31T20:23:15Z

core/src/test/scala/com/microsoft/azure/synapse/ml/nbtest/SynapseTests.scala

+
+  test("listPythonFiles") {
+    val allPythonFiles = SynapseUtilities.listPythonFiles()
+
+    allPythonFiles.foreach(file => println(file))
+  }
+
+  test("listNoteBookFiles") {
+    val allPythonNotebooks = SynapseUtilities.listNoteBookFiles()
+
+    allPythonNotebooks.foreach(file => println(file))
+  }
+
+  test("listPythonJobFiles") {
+    val allPythonJobFiles = SynapseUtilities.listPythonJobFiles()
+
+    allPythonJobFiles.foreach(file => println(file))
+  }


Are these tests that you would like to keep or something that you use for debugging? If the former we might want to add some sort of assert in here so that we can ensure they don't break. If the latter we might want to set them to "ignore" with a comment

I think it is now fine to remove them. Thanks for catching this one!

mhamilton723 · 2022-01-31T20:24:35Z

website/src/pages/index.js

-      "spark.jars.packages": "com.microsoft.azure:synapseml_2.12:0.9.5",
+      "spark.jars.packages": "com.microsoft.azure:synapseml_2.12:0.9.4",


I think this clarification was already added to website

…sts-work

…/riserrad/SynapseML into riserrad/make-synapse-tests-work

riserrad · 2022-02-01T02:50:01Z

/azp run

azure-pipelines · 2022-02-01T02:50:11Z

Azure Pipelines successfully started running 1 pipeline(s).

* Trying to use only pool with Spark 3.2 * Updating install instructions for synapse to use 0.9.4 * Changing syntax to grab ipynb files * Line breaking to comply with styling * Changing ipynb filter for windows * Fixing string new line syntax * Improvements to SynapseTests * Adding more spark pools 3.2 * Adjusting list tests not to assert * Improving dev doc, livyPayLoad * Changing SynapseWS to mmlsparkppe * Changing synapse URL to dogfood * Removing dogfood from token acquisition * Fixing exludes syntax * Adding 2 more Apache Spark Pools * Improving the developer docs * Adjusting identation on developer-readme * Bumping Synapse test timeout to 40 min * Applying PR feedback Co-authored-by: Serena Ruan <82044803+serena-ruan@users.noreply.github.com>

This reverts commit 0840e31.

* Trying to use only pool with Spark 3.2 * Updating install instructions for synapse to use 0.9.4 * Changing syntax to grab ipynb files * Line breaking to comply with styling * Changing ipynb filter for windows * Fixing string new line syntax * Improvements to SynapseTests * Adding more spark pools 3.2 * Adjusting list tests not to assert * Improving dev doc, livyPayLoad * Changing SynapseWS to mmlsparkppe * Changing synapse URL to dogfood * Removing dogfood from token acquisition * Fixing exludes syntax * Adding 2 more Apache Spark Pools * Improving the developer docs * Adjusting identation on developer-readme * Bumping Synapse test timeout to 40 min * Applying PR feedback Co-authored-by: Serena Ruan <82044803+serena-ruan@users.noreply.github.com>

* revert: revert changes of spark 3.2 * fix: change azure-ai-textanalytics dependency to shaded jar and rename namespace to make it compatible with spark 3.1 * allow branch spark3.1 to trigger pipeline * fix shaded jar * fix fasterxml by adding it ahead of coreDependencies * fix io.netty issue * fix io.netty issue * fix databricks conflicts * fix libraries syntax * exclude io.netty:netty-tcnative-boringssl-static * update adbRuntime * exclude org.antlr while installing libraries on dbx clusters * fix adbruntime * fix adbruntime * fix adb runtime * fix adb submit job error * ignore geospatialServices notebooks for adb because adb 9.1 runtime doesn"t support sending http requests to them * fix: Make SynapseE2E Tests work now with Spark 3.2 (#1362) * Trying to use only pool with Spark 3.2 * Updating install instructions for synapse to use 0.9.4 * Changing syntax to grab ipynb files * Line breaking to comply with styling * Changing ipynb filter for windows * Fixing string new line syntax * Improvements to SynapseTests * Adding more spark pools 3.2 * Adjusting list tests not to assert * Improving dev doc, livyPayLoad * Changing SynapseWS to mmlsparkppe * Changing synapse URL to dogfood * Removing dogfood from token acquisition * Fixing exludes syntax * Adding 2 more Apache Spark Pools * Improving the developer docs * Adjusting identation on developer-readme * Bumping Synapse test timeout to 40 min * Applying PR feedback Co-authored-by: Serena Ruan <82044803+serena-ruan@users.noreply.github.com> * change to spark3.1 pools * add more spark pools * Show detailed response of livy * Update url cuz spark3.1 is in prod already * Update SynapseTests.scala * Update SynapseUtilities.scala * fix: remove concurrency parameter for MVAD (#1383) * remove concurrency parameter for MVAD * fix: fix node-fetch version security & error in MVAD sample Co-authored-by: Mark Hamilton <mhamilton723@gmail.com> * fix: expose response error out for better debugging if the error is returned by http directly (#1391) * merge `turn synapse tests into multiple test` Co-authored-by: Ric Serradas <riserrad@microsoft.com> Co-authored-by: Mark Hamilton <mhamilton723@gmail.com>

Trying to use only pool with Spark 3.2

d621a50

riserrad requested a review from mhamilton723 as a code owner January 25, 2022 05:09

riserrad added 3 commits January 24, 2022 21:50

Updating install instructions for synapse to use 0.9.4

280877b

Changing syntax to grab ipynb files

15e3a0b

Line breaking to comply with styling

921f6cf

riserrad added 12 commits January 24, 2022 22:45

Changing ipynb filter for windows

eeece70

Fixing string new line syntax

46f0868

Improvements to SynapseTests

8dfdac3

Adding more spark pools 3.2

aa757b9

Adjusting list tests not to assert

5155083

Improving dev doc, livyPayLoad

a408458

Changing SynapseWS to mmlsparkppe

126e884

Changing synapse URL to dogfood

59d75ca

Removing dogfood from token acquisition

b4bed7e

Excluding slf4j

0583e5e

Fixing exludes syntax

dcd3a6d

Adding 2 more Apache Spark Pools

b7f7c6b

Improving the developer docs

544f1e4

Merge branch 'master' into riserrad/make-synapse-tests-work

814c732

riserrad added 2 commits January 27, 2022 16:53

Adjusting identation on developer-readme

8a56baa

Merge branch 'riserrad/make-synapse-tests-work' of https://github.com…

355eb32

…/riserrad/SynapseML into riserrad/make-synapse-tests-work

Bumping Synapse test timeout to 40 min

6aef918

serena-ruan changed the title ~~Make SynapseE2E Tests work now with Spark 3.2~~ Fix: Make SynapseE2E Tests work now with Spark 3.2 Jan 28, 2022

Merge branch 'master' into riserrad/make-synapse-tests-work

b08e6e2

serena-ruan changed the title ~~Fix: Make SynapseE2E Tests work now with Spark 3.2~~ fix: Make SynapseE2E Tests work now with Spark 3.2 Jan 28, 2022

mhamilton723 requested changes Jan 31, 2022

View reviewed changes

riserrad added 3 commits January 31, 2022 18:43

Merge remote-tracking branch 'upstream' into riserrad/make-synapse-te…

8a743e6

…sts-work

Applying PR feedback

7d8c81b

Merge branch 'riserrad/make-synapse-tests-work' of https://github.com…

ba2cc60

…/riserrad/SynapseML into riserrad/make-synapse-tests-work

mhamilton723 merged commit 0840e31 into microsoft:master Feb 1, 2022

serena-ruan added a commit that referenced this pull request Feb 7, 2022

Revert "fix: Make SynapseE2E Tests work now with Spark 3.2 (#1362)"

bf9dc13

This reverts commit 0840e31.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Make SynapseE2E Tests work now with Spark 3.2 #1362

fix: Make SynapseE2E Tests work now with Spark 3.2 #1362

riserrad commented Jan 25, 2022 •

edited

Loading

riserrad commented Jan 25, 2022

azure-pipelines bot commented Jan 25, 2022

serena-ruan commented Jan 25, 2022

azure-pipelines bot commented Jan 25, 2022

codecov-commenter commented Jan 25, 2022 •

edited

Loading

serena-ruan commented Jan 25, 2022

azure-pipelines bot commented Jan 25, 2022

riserrad commented Jan 27, 2022

azure-pipelines bot commented Jan 27, 2022

svotaw commented Jan 27, 2022

mhamilton723 commented Jan 28, 2022

azure-pipelines bot commented Jan 28, 2022

riserrad commented Jan 28, 2022

azure-pipelines bot commented Jan 28, 2022

riserrad commented Jan 28, 2022

azure-pipelines bot commented Jan 28, 2022

riserrad commented Jan 28, 2022

azure-pipelines bot commented Jan 28, 2022

serena-ruan commented Jan 28, 2022

azure-pipelines bot commented Jan 28, 2022

riserrad commented Jan 28, 2022

azure-pipelines bot commented Jan 28, 2022

riserrad commented Jan 28, 2022

azure-pipelines bot commented Jan 28, 2022

mhamilton723 left a comment

mhamilton723 Jan 31, 2022

riserrad Feb 1, 2022

mhamilton723 Jan 31, 2022

riserrad Feb 1, 2022

riserrad commented Feb 1, 2022

azure-pipelines bot commented Feb 1, 2022

		"spark.jars.packages": "com.microsoft.azure:synapseml_2.12:0.9.5",
		"spark.jars.packages": "com.microsoft.azure:synapseml_2.12:0.9.4",

fix: Make SynapseE2E Tests work now with Spark 3.2 #1362

fix: Make SynapseE2E Tests work now with Spark 3.2 #1362

Conversation

riserrad commented Jan 25, 2022 • edited Loading

Problem

Changes

Results

Opportunities

riserrad commented Jan 25, 2022

azure-pipelines bot commented Jan 25, 2022

serena-ruan commented Jan 25, 2022

azure-pipelines bot commented Jan 25, 2022

codecov-commenter commented Jan 25, 2022 • edited Loading

Codecov Report

serena-ruan commented Jan 25, 2022

azure-pipelines bot commented Jan 25, 2022

riserrad commented Jan 27, 2022

azure-pipelines bot commented Jan 27, 2022

svotaw commented Jan 27, 2022

mhamilton723 commented Jan 28, 2022

azure-pipelines bot commented Jan 28, 2022

riserrad commented Jan 28, 2022

azure-pipelines bot commented Jan 28, 2022

riserrad commented Jan 28, 2022

azure-pipelines bot commented Jan 28, 2022

riserrad commented Jan 28, 2022

azure-pipelines bot commented Jan 28, 2022

serena-ruan commented Jan 28, 2022

azure-pipelines bot commented Jan 28, 2022

riserrad commented Jan 28, 2022

azure-pipelines bot commented Jan 28, 2022

riserrad commented Jan 28, 2022

azure-pipelines bot commented Jan 28, 2022

mhamilton723 left a comment

Choose a reason for hiding this comment

mhamilton723 Jan 31, 2022

Choose a reason for hiding this comment

riserrad Feb 1, 2022

Choose a reason for hiding this comment

mhamilton723 Jan 31, 2022

Choose a reason for hiding this comment

riserrad Feb 1, 2022

Choose a reason for hiding this comment

riserrad commented Feb 1, 2022

azure-pipelines bot commented Feb 1, 2022

riserrad commented Jan 25, 2022 •

edited

Loading

codecov-commenter commented Jan 25, 2022 •

edited

Loading