Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update script to audit multiple Spark versions #539

Merged
merged 10 commits into from
Aug 13, 2020
Merged

Update script to audit multiple Spark versions #539

merged 10 commits into from
Aug 13, 2020

Conversation

nartal1
Copy link
Collaborator

@nartal1 nartal1 commented Aug 11, 2020

This fixes #465.
Added profile to run against different versions.

@nartal1 nartal1 added the feature request New feature or request label Aug 11, 2020
@nartal1 nartal1 added this to the Aug 3 - Aug 14 milestone Aug 11, 2020
@nartal1 nartal1 self-assigned this Aug 11, 2020
@nartal1
Copy link
Collaborator Author

nartal1 commented Aug 11, 2020

I think reviewers were auto added as there was a change in pom file in api_validation repo.
@tgravescs - It would be great if you could take a look and comment if this approach if fine.

@nartal1 nartal1 removed the feature request New feature or request label Aug 11, 2020
@tgravescs
Copy link
Collaborator

build

tgravescs
tgravescs reviewed Aug 11, 2020
View reviewed changes
api_validation/src/main/scala/com/nvidia/spark/rapids/api/ApiValidation.scala Outdated
@@ -69,6 +69,15 @@ object ApiValidation extends Logging {
val gpuKeys = gpuExecs.keys
var printNewline = false

val fullClassName = ShimLoader.getSparkShims.getRapidsShuffleManagerClass
var shimVersion = fullClassName.split('.')(4)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be better to just get the version from the shim itself: getSparkShimVersion

api_validation/pom.xml Outdated
<profile>
<id>spark301</id>
<properties>
<spark.version>3.0.1-SNAPSHOT</spark.version>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have these copied now in a couple places, perhaps we can put a <spark301.version> property in the top level pom file and then just reference it, that way when we change from snapshot to release we only need to change in one place. Same for 3.1.0-SNAPSHOT

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Created PR #549 for above.

@jlowe
Copy link
Member

jlowe commented Aug 11, 2020

Commits need to have signoff.

@nartal1 nartal1 mentioned this pull request Aug 12, 2020
Merged
@nartal1 nartal1 added the feature request New feature or request label Aug 12, 2020
@nartal1
empty commit
b57d578 
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
@nartal1 nartal1 changed the title [WIP] Update script to audit multiple Spark versions Update script to audit multiple Spark versions Aug 12, 2020
kuhushukla
kuhushukla previously approved these changes Aug 12, 2020
View reviewed changes
Copy link
Collaborator

@kuhushukla kuhushukla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good to my shim-naive eyes. We can have Tom take a final look but lgtm !

@nartal1
Copy link
Collaborator Author

nartal1 commented Aug 13, 2020

build

tgravescs
tgravescs reviewed Aug 13, 2020
View reviewed changes
api_validation/README.md Outdated
@@ -15,7 +17,11 @@ It requires cudf, rapids-4-spark and spark jars.

```
cd api_validation
mvn scala:run
// To run validation script on all version of Spark-3(3.0.0, 3.0.1-SNAPSHOT and 3.1.0-SNAPSHOT)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you mean to have the -3 on here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to mean it as different versions of Spark 3. Removed it as it might be confusing.

tgravescs
tgravescs reviewed Aug 13, 2020
View reviewed changes
api_validation/src/main/scala/com/nvidia/spark/rapids/api/ApiValidation.scala Outdated
@@ -69,6 +69,16 @@ object ApiValidation extends Logging {
val gpuKeys = gpuExecs.keys
var printNewline = false

val sparkToShimMap = Map("3.0.0"->"spark300", "3.0.1"->"spark301", "3.1.0"->"spark310")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit add spaces around the -> on both sides.

api_validation/src/main/scala/com/nvidia/spark/rapids/api/ApiValidation.scala Show resolved Hide resolved
@nartal1
addressed review comments
53eaefa 
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
@nartal1
addresed review comments
b91ce4f 
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
tgravescs
tgravescs approved these changes Aug 13, 2020
View reviewed changes
Copy link
Collaborator

@tgravescs tgravescs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good pending Jenkins

@tgravescs
Copy link
Collaborator

build

@nartal1 nartal1 merged commit afbb613 into NVIDIA:branch-0.2 Aug 13, 2020
nartal1 added a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@nartal1
Update script to audit multiple Spark versions (NVIDIA#539)
e32e194 
* Update audit script to support multiple Spark versions

* Update readme and validation script

* addressed review comments

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* update pom

* empty commit

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* addressed review comments

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* addresed review comments

Signed-off-by: Niranjan Artal <nartal@nvidia.com>
nartal1 added a commit to nartal1/spark-rapids that referenced this pull request Jun 9, 2021
@nartal1
Update script to audit multiple Spark versions (NVIDIA#539)
7017295 
* Update audit script to support multiple Spark versions

* Update readme and validation script

* addressed review comments

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* update pom

* empty commit

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* addressed review comments

Signed-off-by: Niranjan Artal <nartal@nvidia.com>

* addresed review comments

Signed-off-by: Niranjan Artal <nartal@nvidia.com>
tgravescs pushed a commit to tgravescs/spark-rapids that referenced this pull request Nov 30, 2023
@nvauto
Update submodule cudf to 9f8db66 (NVIDIA#539)
3ca1c1a 
Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>

Signed-off-by: spark-rapids automation <70000568+nvauto@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgravescs tgravescs tgravescs approved these changes

@kuhushukla kuhushukla kuhushukla left review comments

@GaryShen2008 GaryShen2008 Awaiting requested review from GaryShen2008 GaryShen2008 is a code owner

@jlowe jlowe Awaiting requested review from jlowe jlowe is a code owner

@NvTimLiu NvTimLiu Awaiting requested review from NvTimLiu NvTimLiu is a code owner

@revans2 revans2 Awaiting requested review from revans2 revans2 is a code owner

Assignees

@nartal1 nartal1

Labels
feature request New feature or request
Projects
None yet
Milestone
Aug 3 - Aug 14
Development

Successfully merging this pull request may close these issues.

[FEA] Audit: Update script to audit multiple versions of Spark
4 participants
@nartal1 @tgravescs @jlowe @kuhushukla