Skip to content

Conversation

@michal-wieleba
Copy link
Contributor

@michal-wieleba michal-wieleba commented Aug 21, 2020

What changes were proposed in this pull request?

Removing unused DELETE_ACTION in FileStreamSinkLog.

Why are the changes needed?

DELETE_ACTION is not used nowhere in the code.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Tests where not added, because code was removed.

@HeartSaVioR HeartSaVioR changed the title [SPARK-32648][Structured Streaming] Remove unused DELETE_ACTION in FileStreamSinkLog [SPARK-32648][SS] Remove unused DELETE_ACTION in FileStreamSinkLog Aug 22, 2020
@HeartSaVioR
Copy link
Contributor

ok to test

@HeartSaVioR
Copy link
Contributor

I'm OK with the changes, but given the idea was from me, it would be safer to review from others as well.

cc. @dongjoon-hyun @gatorsmile

@SparkQA
Copy link

SparkQA commented Aug 22, 2020

Test build #127779 has finished for PR 29505 at commit e50fddb.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@dongjoon-hyun
Copy link
Member

Thank you for pinging me, @HeartSaVioR . Yes, I agree with you.
It looks function-wise safe inside Apache Spark, but deleting something needs more wider agreement.

cc @tdas , @zsxwing , @marmbrus , @cloud-fan , too.

@zsxwing
Copy link
Member

zsxwing commented Aug 22, 2020

Removing it sounds good to me. After #28904, we won't be able to support DELETE_ACTION without upgrading the log version. If we really need this, we can add it back with a new log format.

@dongjoon-hyun
Copy link
Member

Thank you, @zsxwing . Merged to master for Apache Spark 3.1.0 on December 2020.

@dongjoon-hyun
Copy link
Member

Thank you and welcome, @michal-wieleba .
You are added to the Apache Spark contributor group and SPARK-32648 is assigned to you.

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