Skip to content

[SPARK-43299][FOLLOWUP][CONNECT][SS] Followup on StreamingQueryExceptions#44306

Closed
WweiL wants to merge 8 commits into
apache:masterfrom
WweiL:SPARK-43299-followup
Closed

[SPARK-43299][FOLLOWUP][CONNECT][SS] Followup on StreamingQueryExceptions#44306
WweiL wants to merge 8 commits into
apache:masterfrom
WweiL:SPARK-43299-followup

Conversation

@WweiL

@WweiL WweiL commented Dec 12, 2023

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Follow up on streaming query exception in Scala connect client.

  1. Change back API document, remove the "todo"s
  2. Add a new test for the streamingQueryManager's awaitAnyTermination API
  3. Slightly modified the constructor of StreamingQueryException by adding the queryDebugString, startOffset and endOffset to messageParameters, this enables us to reconstruct the exception with these fields in client side.

Why are the changes needed?

Necessary build ups for spark connect.

Does this PR introduce any user-facing change?

No

How was this patch tested?

New Unit test

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

No

@WweiL

WweiL commented Dec 12, 2023

Copy link
Copy Markdown
Contributor Author

@MaxGekk @HyukjinKwon Tagging people that I know who have context : )
Could you take a look when you have time? Thank you!

@HyukjinKwon

Copy link
Copy Markdown
Member

Looks good but I think it'd be great if this can be signed off by @MaxGekk

@MaxGekk MaxGekk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@WweiL Could you trigger GAs, please.

@WweiL

WweiL commented Dec 12, 2023

Copy link
Copy Markdown
Contributor Author

Use empty string as default for test failures
https://github.com/WweiL/oss-spark/actions/runs/7186087215/job/19570752262

@WweiL WweiL requested a review from MaxGekk December 12, 2023 21:57
@WweiL

WweiL commented Dec 14, 2023

Copy link
Copy Markdown
Contributor Author

@MaxGekk Hi Max can I get another pass? Thank you!

@heyihong heyihong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with one minor comment

@HyukjinKwon

Copy link
Copy Markdown
Member

Merged to master.

Comment on lines +333 to +335
"queryDebugString" -> toDebugString(includeLogicalPlan = isInitialized),
"startOffset" -> committedOffsets.toOffsetSeq(sources, offsetSeqMetadata).toString,
"endOffset" -> availableOffsets.toOffsetSeq(sources, offsetSeqMetadata).toString

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When you pass additional parameters, you should add placeholders for them in the error formats in error-conditions.json otherwise they are useless. Found them in the PR: #48026

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.

4 participants