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

added deletion of state for Functions #5469

Merged

Conversation

@johnwagster
Copy link
Contributor

johnwagster commented Oct 25, 2019

Motivation

Adding support for state deletion for Functions. Ran into a use-case where it would be nice to be able to let go of state wherein the key is changing but the data itself is ephemeral.

Modifications

I've reflected the changes in the bookkeepr API back through in the context and state context API's so that you can delete on a put by passing null or call the new api for delete.

Verifying this change

This change added tests and can be verified as follows:

  • Running the unit tests available in the ContextImplTest and ContextStateImplTest

Does this pull request potentially affect one of the following parts:

  • The public API: yes; added deletion to Function context and state context APIs

Documentation

  • Does this pull request introduce a new feature? - yes
  • If yes, how is the feature documented? - JavaDocs
Copy link
Contributor

jerrypeng left a comment

@johnwagster thanks for the contribution.

@jerrypeng

This comment has been minimized.

Copy link
Contributor

jerrypeng commented Oct 25, 2019

rerun integration tests

@jerrypeng

This comment has been minimized.

Copy link
Contributor

jerrypeng commented Oct 25, 2019

@johnwagster it would be great if you can also add the delete functionality to the pulsar-admin CLI and a corresponding REST endpoint!

@johnwagster

This comment has been minimized.

Copy link
Contributor Author

johnwagster commented Oct 25, 2019

I'd be happy to add state delete to the admin cli and rest endpoints. Would you like to see that in this PR or is a separate PR alright?

@sijie
sijie approved these changes Oct 27, 2019
@sijie

This comment has been minimized.

Copy link
Contributor

sijie commented Oct 27, 2019

Would you like to see that in this PR or is a separate PR alright?

A separate PR is good.

// run integration tests

@jiazhai jiazhai merged commit 309c78e into apache:master Oct 28, 2019
3 checks passed
3 checks passed
Jenkins: C++ / Python Tests SUCCESS
Details
Jenkins: Integration Tests SUCCESS
Details
Jenkins: Java 8 - Unit Tests SUCCESS
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.