Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove "name()" from StateBuilder interface #1988

Merged
merged 1 commit into from
Jan 19, 2018
Merged

Remove "name()" from StateBuilder interface #1988

merged 1 commit into from
Jan 19, 2018

Conversation

jtfmumm
Copy link
Contributor

@jtfmumm jtfmumm commented Jan 15, 2018

We don't currently use the StateBuilder name for any
reason and it creates more boilerplate when creating
stateful apps. This removes name() from the interface
and updates Python and Go APIs as well as example
apps and book examples accordingly. Instead of the
StateBuilder name, we now use the state partition
name, which would do the trick if we ever end up
needing it for logging purposes.

Closes #1809

@jtfmumm
Copy link
Contributor Author

jtfmumm commented Jan 15, 2018

@aturley I've put you on this PR because it's mostly changes to the Python and Go APIs as well as related examples. The core changes are removing name() from the StateBuilder interface and using the state partition name where we'd previously used that method internally.

@SeanTAllen
Copy link
Contributor

Given that this is completely invisible to end users, I think the CHANGELOG label can be removed. There's no need for this to be in the CHANGELOG.

@jtfmumm
Copy link
Contributor Author

jtfmumm commented Jan 19, 2018

@SeanTAllen This required changes to the Go and Pony example apps since it removed the requirement for name(). That's not invisible. Note that @aturley included that method in his latest blog post on the Go API since it was a requirement. Of course, the example apps would have still compiled since this reduces the requirements for the interface, but it's no longer necessary and serves no purpose to implement that method after this change.

@SeanTAllen
Copy link
Contributor

Ah, ok. This is however, a non-breaking change, correct?

I'm looking at https://github.com/WallarooLabs/wallaroo/blob/428949bec59d7682919ddd8a66bf0a6c477ca78d/book/python/writing-your-own-partitioned-stateful-application.md

and it seems to have not been updated for the new API.

@jtfmumm
Copy link
Contributor Author

jtfmumm commented Jan 19, 2018

@SeanTAllen This is a non-breaking change. I don't see what I missed in that document you linked to.

@jtfmumm jtfmumm force-pushed the i1809 branch 2 times, most recently from 1bc16ef to 2a5dfcf Compare January 19, 2018 13:43
We don't currently use the StateBuilder name for any
reason and it creates more boilerplate when creating
stateful apps. This removes name() from the interface
and updates Python and Go APIs as well as example
apps and book examples accordingly. Instead of the
StateBuilder name, we now use the state partition
name, which would do the trick if we ever end up
needing it for logging purposes.

[skip ci]
Closes #1809
Copy link
Contributor

@aturley aturley left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@aturley aturley merged commit 3c448ea into master Jan 19, 2018
wallaroolabs-wally added a commit that referenced this pull request Jan 19, 2018
@aturley aturley deleted the i1809 branch January 19, 2018 14:27
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.

3 participants