Skip to content

Conversation

@tinglongliao-db
Copy link
Contributor

@tinglongliao-db tinglongliao-db commented Oct 15, 2024

What changes were proposed in this pull request?

Previously, the output deprecation warning happens in the spark.session function, in this PR, we move it to the .onAttach function so it will be triggered whenever library is attached

Why are the changes needed?

I believe having the warning message on attach time have the following benefits:

  • Have a more prompt warning. If the deprecation is for the whole package instead of just the sparkR.session function, it is more intuitive for the warning to show up on attach time instead of waiting til later time
  • Do not rely on the assumption of "every sparkR user will run sparkR.session method". This asumption may not hold true all the time. For example, some hosted spark platform like Databricks already configure the spark session in the background and therefore will not show the error message. So making this change should make sure a broader reach for this warning notification
  • Less intrusive warning. Previous warning show up every time sparkR.session is called, but the new warning message will only show up once even if user run multiple library/require commands

Does this PR introduce any user-facing change?

Yes

  1. No more waring message in sparkR.session method
  2. Warning message on library attach (when calling library/require function)
image 3. Able to surpress warning by setting `SPARKR_SUPPRESS_DEPRECATION_WARNING` image

How was this patch tested?

Just a simple migration change, will rely on existing pre/post-merge check, and this existing test
Also did manual testing(see previous section for screenshot)

Was this patch authored or co-authored using generative AI tooling?

No

@github-actions github-actions bot added the R label Oct 15, 2024
@tinglongliao-db tinglongliao-db changed the title Move sparkR deprecation warning to package attach time [SPARK-49978][R]Move sparkR deprecation warning to package attach time Oct 15, 2024
@tinglongliao-db tinglongliao-db force-pushed the sparkR-deprecation-migration branch from 046e2c1 to fa18311 Compare October 15, 2024 20:09
@dongjoon-hyun
Copy link
Member

cc @HyukjinKwon

@HyukjinKwon
Copy link
Member

Merged to master.

@HyukjinKwon
Copy link
Member

(Just for context, I already discussed offline - this way it's better for the end users to see the deprecation warning, and how it's deprecated is similar with other standard R packages)

@tinglongliao-db tinglongliao-db deleted the sparkR-deprecation-migration branch October 16, 2024 23:14
@dongjoon-hyun
Copy link
Member

Thank you, @tinglongliao-db and @HyukjinKwon .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants