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

create persistent sequential node with empty bytes #197

Merged
merged 3 commits into from
Oct 31, 2016
Merged

Conversation

ssalinas
Copy link
Member

@tpetr ObjectMapper will still throw an error if it's given empty bytes. So I modified the readFromZk method to check for data length as well as updating the persistent sequential to be created with empty bytes. If we get empty bytes we return an absent

Context for others:

  • When curator creates a persistent sequential node with no argument for content/data, it defaults the data to the ip of the client creating it. It was possible that BaragonService would try to read this in between the creation of the node and setting the data in the addAgentResponse method in the BaragonAgentResponseDatastore

@tpetr
Copy link
Contributor

tpetr commented Oct 28, 2016

LGTM

@tpetr
Copy link
Contributor

tpetr commented Oct 28, 2016

I like that we're explicit with what data to set, but I also wonder if we should just override Curator's default value, so that we don't possibly get bitten by this in other places too?

@ssalinas ssalinas added the qa label Oct 28, 2016
@ssalinas
Copy link
Member Author

@tpetr updated the defaultData to new byte[] {} in the BaragonServiceModule

.sessionTimeoutMs(config.getSessionTimeoutMillis())
.connectionTimeoutMs(config.getConnectTimeoutMillis())
.retryPolicy(new ExponentialBackoffRetry(config.getRetryBaseSleepTimeMilliseconds(), config.getRetryMaxTries()))
.defaultData(new byte[] {})
Copy link
Member

Choose a reason for hiding this comment

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

nit: new byte[0]?

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 👍

@ssalinas ssalinas merged commit d818810 into master Oct 31, 2016
@ssalinas ssalinas deleted the sequential_nodes branch October 31, 2016 15:02
@ssalinas ssalinas removed the staging label Oct 31, 2016
@ssalinas ssalinas modified the milestone: 0.5.0 May 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants