Skip to content

Conversation

@xushiyan
Copy link
Member

@xushiyan xushiyan commented Jul 6, 2020

  • Assertions.java for common assertions
  • refactor assertNoWriteErrors()

Committer checklist

  • Has a corresponding JIRA in PR title & commit

  • Commit message is descriptive of the change

  • CI is green

  • Necessary doc changes done or have another open PR

  • For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

@xushiyan xushiyan changed the title [HUDI-1069] Add Assertions helper class [HUDI-1069] Remove duplicate assertNoWriteErrors() Jul 6, 2020
@xushiyan xushiyan force-pushed the add-assertions-helpers branch from 075c563 to c05f781 Compare July 6, 2020 07:18
@yanghua yanghua self-assigned this Jul 6, 2020
@yanghua yanghua self-requested a review July 6, 2020 15:45
@xushiyan xushiyan force-pushed the add-assertions-helpers branch from 25cf4ac to cd4d5f2 Compare July 6, 2020 19:38
@xushiyan
Copy link
Member Author

xushiyan commented Jul 7, 2020

@yanghua this is ready for review. Thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @xushiyan I have a concern about open this method. Currently, @wangxianghu is refactoring client module #1727 . He is trying to decouple the dependency with Spark APIs. Maybe we'd better have this interface:

  public static void assertNoWriteErrors(List<WriteStatus> statuses) {
    //...
  }

WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok that method is also there. just need to remove the overloading version for RDD. changing it now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated. Thanks @yanghua

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for addressing my concern.

- Assertions.java for common assertions
- refactor assertNoWriteErrors()
@xushiyan xushiyan force-pushed the add-assertions-helpers branch from cd4d5f2 to fcd087b Compare July 8, 2020 02:23
Copy link
Contributor

@yanghua yanghua left a comment

Choose a reason for hiding this comment

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

@xushiyan Thanks for your contribution. LGTM.

@yanghua yanghua merged commit 7b2a947 into apache:master Jul 8, 2020
@xushiyan xushiyan deleted the add-assertions-helpers branch August 8, 2020 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants