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

[SPARK-50770][SS] Removing package scope for transformWithState operator APIs #49417

Closed
wants to merge 5 commits into from

Conversation

ericm-db
Copy link
Contributor

@ericm-db ericm-db commented Jan 8, 2025

What changes were proposed in this pull request?

Removing package scope for transformWithState operator APIs

This enables us to release the transformWithState APIs from the next Spark release. While traits and methods expand visibility, the usage is blocked due to a flag. The flag will be flipped in separate PR and corresponding behavior change ticket will be filed with it.

Why are the changes needed?

To allow use of the transformWithState operator

Does this PR introduce any user-facing change?

Yes

How was this patch tested?

Existing unit tests

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

No

@ericm-db ericm-db changed the title [WIP] Enable TWS [SPARK-50770] Removing package scope for transformWithState operator APIs Jan 8, 2025
@HyukjinKwon HyukjinKwon changed the title [SPARK-50770] Removing package scope for transformWithState operator APIs [SPARK-50770][SS] Removing package scope for transformWithState operator APIs Jan 9, 2025
@dongjoon-hyun
Copy link
Member

Gentle ping, @ericm-db . Please address the above review comments.

@HeartSaVioR
Copy link
Contributor

NOTE: PLEASE DO NOT MERGE THIS PR. WE HAVE TO ADDRESS SPARK CONNECT WHICH WILL BE COMING SOON.

@HeartSaVioR
Copy link
Contributor

@ericm-db Could you please change the title to add [DO-NOT-MERGE] in prefix?

Copy link
Contributor

@HeartSaVioR HeartSaVioR left a comment

Choose a reason for hiding this comment

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

I'll make this be explicit again, this depends on Spark Connect integration work. Before that, do not merge.

@ericm-db ericm-db changed the title [SPARK-50770][SS] Removing package scope for transformWithState operator APIs [DO NOT MERGE][SPARK-50770][SS] Removing package scope for transformWithState operator APIs Jan 16, 2025
@anishshri-db
Copy link
Contributor

this depends on Spark Connect integration work. Before that, do not merge.

We discussed this offline and we are not going to treat connect work as a blocker for 4.0. We have the Scala PR for connect support in review - #49488 and we will work on the Python PR too. But we will treat them as best effort for 4.0.

@ericm-db ericm-db changed the title [DO NOT MERGE][SPARK-50770][SS] Removing package scope for transformWithState operator APIs [SPARK-50770][SS] Removing package scope for transformWithState operator APIs Jan 18, 2025
@anishshri-db
Copy link
Contributor

@HeartSaVioR - PTAL when u get a chance, thx !

Copy link
Contributor

@HeartSaVioR HeartSaVioR left a comment

Choose a reason for hiding this comment

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

+1 pending CI (I think it isn't necessary, but just to make 100% sure)

@anishshri-db
Copy link
Contributor

@HeartSaVioR - CI is green. Not updating above. Just fyi

@HeartSaVioR
Copy link
Contributor

Thanks! Merging to master/4.0.

HeartSaVioR pushed a commit that referenced this pull request Jan 24, 2025
…tor APIs

### What changes were proposed in this pull request?

Removing package scope for transformWithState operator APIs

This enables us to release the transformWithState APIs from the next Spark release. While traits and methods expand visibility, the usage is blocked due to a flag. The flag will be flipped in separate PR and corresponding behavior change ticket will be filed with it.

### Why are the changes needed?

To allow use of the transformWithState operator

### Does this PR introduce _any_ user-facing change?

Yes

### How was this patch tested?

Existing unit tests

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

No

Closes #49417 from ericm-db/enable-tws.

Authored-by: Eric Marnadi <[email protected]>
Signed-off-by: Jungtaek Lim <[email protected]>
(cherry picked from commit be77e1e)
Signed-off-by: Jungtaek Lim <[email protected]>
@dongjoon-hyun
Copy link
Member

Thank you all for this!

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

Successfully merging this pull request may close these issues.

5 participants