Skip to content

Conversation

@dnhatn
Copy link
Member

@dnhatn dnhatn commented Jan 30, 2019

Since #31140 we no longer require acking on the dynamic mapping of index requests. Thus, a returned mapping from a get mapping request does not necessarily contain the dynamic updates from the index request. This commit replaces the dynamic mapping update with a manual put mapping.

Relates #31140
Closes #37928

We need to await busily for the mapping in this test because we no
longer require acking on the dynamic mapping of an index request.

Relates elastic#31140
Closes elastic#37928
@dnhatn dnhatn added >test Issues or PRs that are addressing/adding tests :Search Foundations/Mapping Index mappings, including merging and defining field types v7.0.0 v6.7.0 labels Jan 30, 2019
@dnhatn dnhatn requested a review from martijnvg January 30, 2019 18:30
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

assertNotNull(response.fieldMappings("test", "type", "obj.subfield"));
assertThat((Map<String, Object>) response.fieldMappings("test", "type", "obj.subfield").sourceAsMap().get("subfield"),
hasEntry("type", "keyword"));
});
Copy link
Member

Choose a reason for hiding this comment

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

I was wondering if the whole code block needs to be in the assertBusy ?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, we only need to ensure that the mapping is ready. Anyway, I replaced the dynamic mapping update with a put-mapping request. Thanks for looking @javanna.

@dnhatn dnhatn changed the title AssertBusy in testSimpleGetFieldMappingsWithDefaults Disable dynamic mapping in testSimpleGetFieldMappingsWithDefaults Jan 31, 2019
@dnhatn dnhatn requested review from jtibshirani and removed request for martijnvg January 31, 2019 17:39
@dnhatn
Copy link
Member Author

dnhatn commented Jan 31, 2019

run elasticsearch-ci/default-distro

Copy link
Contributor

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

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

This looks good to me!

@dnhatn
Copy link
Member Author

dnhatn commented Feb 1, 2019

Thanks @javanna and @jtibshirani.

@dnhatn dnhatn merged commit b8b8434 into elastic:master Feb 1, 2019
@dnhatn dnhatn deleted the assert-busy-field-mappings branch February 1, 2019 02:01
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Feb 1, 2019
…astic#38045)

Since elastic#31140 we no longer require acking on the dynamic mapping of index
requests. Thus, a returned mapping from a get mapping request does not
necessarily contain the dynamic updates from the index request. This
commit replaces the dynamic mapping update with a manual put mapping.

Relates elastic#31140
Closes elastic#37928
dnhatn added a commit that referenced this pull request Feb 1, 2019
Since #31140 we no longer require acking on the dynamic mapping of index
requests. Thus, a returned mapping from a get mapping request does not
necessarily contain the dynamic updates from the index request. This
commit replaces the dynamic mapping update with a manual put mapping.

Relates #31140
Closes #37928
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Search Foundations/Mapping Index mappings, including merging and defining field types >test Issues or PRs that are addressing/adding tests v6.7.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants