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

Report conflict when trying to disable _ttl #7316

Closed
wants to merge 4 commits into from

Conversation

brwe
Copy link
Contributor

@brwe brwe commented Aug 18, 2014

_ttl could never be disabled once it was enabled.
But when trying to, no conflict was reported.

relates to #777 and #7293

Also, add a line about that it can never be disabled after it was enabled.
_ttl could never be disabled once it was enabled.
But when trying to, no conflict was reported.

relates to elastic#777 and elastic#7293
@@ -238,12 +239,14 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
@Override
public void merge(Mapper mergeWith, MergeContext mergeContext) throws MergeMappingException {
TTLFieldMapper ttlMergeWith = (TTLFieldMapper) mergeWith;
if (!mergeContext.mergeFlags().simulate()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you should not ignore it completely? The contract is that is simulate is true then conflicts are reported but changes are not performed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! Thank god for code reviews...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a commit and tests for this that were missing before

@jpountz jpountz removed the review label Aug 18, 2014
@brwe
Copy link
Contributor Author

brwe commented Aug 19, 2014

Adressed all comments.

@jpountz
Copy link
Contributor

jpountz commented Aug 20, 2014

LGTM

@jpountz jpountz removed the review label Aug 20, 2014
@brwe brwe closed this in ab9e33e Aug 21, 2014
brwe added a commit that referenced this pull request Aug 21, 2014
_ttl could never be disabled once it was enabled.
But when trying to, no conflict was reported.

relates to #777 and #7293

closes #7316
@spinscale spinscale added the bug label Aug 26, 2014
brwe added a commit that referenced this pull request Sep 8, 2014
_ttl could never be disabled once it was enabled.
But when trying to, no conflict was reported.

relates to #777 and #7293

closes #7316
@clintongormley clintongormley changed the title _ttl: Report conflict when trying to disable _ttl Mapping: Report conflict when trying to disable _ttl Sep 8, 2014
@clintongormley clintongormley added the :Search/Mapping Index mappings, including merging and defining field types label Jun 7, 2015
@clintongormley clintongormley changed the title Mapping: Report conflict when trying to disable _ttl Report conflict when trying to disable _ttl Jun 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Mapping Index mappings, including merging and defining field types v1.4.0.Beta1 v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants