Repository navigation
Conversation
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
+1, LGTM.
BTW, if you want to backport this to branch-4.x for Spark 4.3.0, you had better get a new JIRA ID.
Otherwise, this is only for Apache Spark 5.0.0 (master).
|
Thank you @dongjoon-hyun! Merging to master only |
|
Merge Summary:
Posted by |
|
Hi @dongjoon-hyun , I'm good with holding a decent bar of creating JIRA tickets instead of using This PR should be merged to 4.x. For creating JIRA ticket or not for this change, I don't have a strong opinion but if a committer raises a concern then we should always create one. @uros-b can you create a 4.x backport? |
|
Thank you @cloud-fan, let me backport to 4.x as well |
…ltering ### What changes were proposed in this pull request? Replaces `filter(_.isSnapshot == false)` with the equivalent, idiomatic `filterNot(_.isSnapshot)` in `HDFSBackedStateStoreProvider.doSnapshot`. ### Why are the changes needed? `StoreFile.isSnapshot` is a plain `Boolean` field, so comparing it to `false` is redundant. `filterNot` is the idiomatic form used throughout Spark and reads more clearly. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Behavior-preserving one-line change; existing state-store tests cover the path. No new tests needed. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8) Closes #57519 from uros-b/cleanup-hdfs-state-filternot. Lead-authored-by: Uros Bojanic <[email protected]> Co-authored-by: Uros Bojanic <[email protected]> Signed-off-by: Uros Bojanic <[email protected]> (cherry picked from commit fd31694) Signed-off-by: Uros Bojanic <[email protected]>
|
Merge Summary:
Posted by |
|
Backported to |
What changes were proposed in this pull request?
Replaces
filter(_.isSnapshot == false)with the equivalent, idiomaticfilterNot(_.isSnapshot)inHDFSBackedStateStoreProvider.doSnapshot.Why are the changes needed?
StoreFile.isSnapshotis a plainBooleanfield, so comparing it tofalseis redundant.filterNotis the idiomatic form used throughout Spark and reads more clearly.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Behavior-preserving one-line change; existing state-store tests cover the path. No new tests needed.
Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)