-
Notifications
You must be signed in to change notification settings - Fork 235
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
Prevent bad practice in python tests #482
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Robert (Bobby) Evans <[email protected]>
build |
kuhushukla
previously approved these changes
Jul 31, 2020
tgravescs
reviewed
Jul 31, 2020
|
||
def get_spark_i_know_what_i_am_doing(): | ||
""" | ||
Get the current SparkSession. Because of how tests work |
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.
it might be nice to state why
build |
@kuhushukla and @tgravescs could you take another look? |
kuhushukla
approved these changes
Jul 31, 2020
kuhushukla
approved these changes
Jul 31, 2020
tgravescs
approved these changes
Jul 31, 2020
nartal1
pushed a commit
to nartal1/spark-rapids
that referenced
this pull request
Jun 9, 2021
* Prevent bad practice in python tests Signed-off-by: Robert (Bobby) Evans <[email protected]> * Addressed review comments
nartal1
pushed a commit
to nartal1/spark-rapids
that referenced
this pull request
Jun 9, 2021
* Prevent bad practice in python tests Signed-off-by: Robert (Bobby) Evans <[email protected]> * Addressed review comments
pxLi
pushed a commit
to pxLi/spark-rapids
that referenced
this pull request
May 12, 2022
tgravescs
pushed a commit
to tgravescs/spark-rapids
that referenced
this pull request
Nov 30, 2023
Signed-off-by: spark-rapids automation <[email protected]> Signed-off-by: spark-rapids automation <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In general trying to access the spark session outside of a
with_*_session
can result in tests that don't do what you want them to do. To help prevent this from happening this makesspark
which was available for everyone to use before now_spark
which shows that it is intended to be private. It also adds a check when returning something from awith_*_session
to try and avoid someone returning a dataframe. When you create a dataframe with one config and then change configs spark has some lazy processing and you don't always get what you expect in terms of how that data was configured.