Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

[Java Streamlet API] Extend validations #3095

Merged

Conversation

erenavsarogullari
Copy link
Member

This PR aims to add more validations

  • BuilderImpl level to fail fast with meaningful error message
  • Streamlet clone function should accept as numClones > 0

@@ -44,6 +46,7 @@ public BuilderImpl() {

@Override
public <R> Streamlet<R> newSource(SerializableSupplier<R> supplier) {
Preconditions.checkNotNull(supplier, "supplier must not be null.");
Copy link
Contributor

Choose a reason for hiding this comment

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

why use Preconditions? require() is not enough?

We are hoping to get rid of guava library since it is too big. :(

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, require can be used by moving from StreamletImpl to StreamletUtils as the current patch

@@ -216,7 +216,7 @@ protected StreamletImpl() {

public void build(TopologyBuilder bldr, Set<String> stageNames) {
if (built) {
throw new RuntimeException("Logic Error While building " + getName());
throw new RuntimeException("Logic Error While building stage: " + getName());
Copy link
Contributor

Choose a reason for hiding this comment

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

why "stage"?

Copy link
Contributor

@nwangtw nwangtw left a comment

Choose a reason for hiding this comment

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

lgtm~

@erenavsarogullari
Copy link
Member Author

@nwangtw @huijunwu
Thanks for reviews. It is ready to be merged.

@nlu90 nlu90 merged commit 67af633 into apache:master Nov 5, 2018
@erenavsarogullari erenavsarogullari deleted the HeronPR_StreamletAPI_Validation branch November 5, 2018 21:31
nicknezis pushed a commit that referenced this pull request Sep 14, 2020
* Add validations to Streamlet API

* Review comments are addressed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants