Skip to content

Expose WriteRequest.RefreshPolicy string representation#23106

Merged
tlrx merged 3 commits intoelastic:masterfrom
tlrx:update-refresh-policy
Feb 13, 2017
Merged

Expose WriteRequest.RefreshPolicy string representation#23106
tlrx merged 3 commits intoelastic:masterfrom
tlrx:update-refresh-policy

Conversation

@tlrx
Copy link
Copy Markdown
Member

@tlrx tlrx commented Feb 10, 2017

This commit changes the RefreshPolicy enum so that string representation are exposed. This will help the high level rest client to simply use refreshPolicy.getValue() to get the corresponding parameter value of a given refresh policy.

This commit changes the RefreshPolicy enum so that string representation are exposed. This will help the high level rest client to simply use  refreshPolicy.getValue() to get the corresponding parameter value of a given refresh policy.
Copy link
Copy Markdown
Contributor

@javanna javanna left a comment

Choose a reason for hiding this comment

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

left a couple of comments

* Don't refresh after this request. The default.
*/
NONE,
NONE((byte) 0, Boolean.FALSE.toString()),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I am wondering if using "true" and "false" would be more readable, simply because I always forget if it's lowercase or uppercase.

WAIT_UNTIL;
WAIT_UNTIL((byte) 2, "wait_for");

private final byte id;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we remove the id member and stick with the enum ordinal like we were doing before?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes we can :(

return policy;
}
}
throw new IllegalArgumentException("No cluster block level matching [" + id + "]");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this a cluster block level?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's a wonderful copy paste

@tlrx
Copy link
Copy Markdown
Member Author

tlrx commented Feb 10, 2017

Thanks @javanna, I updated the code.

public void writeTo(StreamOutput out) throws IOException {
out.writeByte(getId());
}
out.writeByte((byte) ordinal()); }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: } is in a funny place.

Copy link
Copy Markdown
Contributor

@javanna javanna left a comment

Choose a reason for hiding this comment

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

LGTM besides nik's comment

@tlrx tlrx merged commit de94c12 into elastic:master Feb 13, 2017
@tlrx tlrx deleted the update-refresh-policy branch February 13, 2017 09:49
@tlrx
Copy link
Copy Markdown
Member Author

tlrx commented Feb 13, 2017

Thanks @javanna @nik9000

@tlrx tlrx removed the review label Feb 13, 2017
tlrx added a commit that referenced this pull request Feb 13, 2017
This commit changes the RefreshPolicy enum so that string representation are exposed. This will help the high level rest client to simply use  refreshPolicy.getValue() to get the corresponding parameter value of a given refresh policy.
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