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.

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

fix(datasets): Fix spark tests #773

Closed
wants to merge 1 commit into from

Conversation

felipemonroy
Copy link
Contributor

@felipemonroy felipemonroy commented Jul 20, 2024

Description

Spark tests are failing randomly. This could be due that spark sessions are not properly closed, so some tests are using the one that do not correspond. See #755

Development notes

I replaced all SparkSession.builder.getOrCreate() calls for the fixture spark session, so we can control which spark session is used. Additionally, in the tear down of the spark session fixtures I added spark._instantiatedContext = None, which allegedly ensure that another spark session is created after (see here).

I am not 100% sure that this is going to fix the issue. Maybe we can rerun the actions a few time just to be sure.

Checklist

  • Opened this PR as a 'Draft Pull Request' if it is work-in-progress
  • Updated the documentation to reflect the code changes
  • Added a description of this change in the relevant RELEASE.md file
  • Added tests to cover my changes

Signed-off-by: Felipe Monroy <felipe.m02@gmail.com>
@felipemonroy felipemonroy marked this pull request as ready for review July 20, 2024 21:19
@felipemonroy felipemonroy mentioned this pull request Jul 22, 2024
4 tasks
@ankatiyar
Copy link
Contributor

Hey @felipemonroy, first of all, thank you so much for taking this on! 🙏🏾
I re-ran the unit tests and it failed with the same errors for the spark tests https://github.com/kedro-org/kedro-plugins/actions/runs/10023089947/job/27738930808
If you need any help, let us know

@felipemonroy felipemonroy marked this pull request as draft July 22, 2024 12:48
@astrojuanlu
Copy link
Member

Should we add .enableHiveSupport(), like it's done in another fixture?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants