Skip to content

fix for broken template mapping json #142

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

Merged
merged 4 commits into from
Jun 8, 2020

Conversation

Munawwar
Copy link
Contributor

@Munawwar Munawwar commented Jun 3, 2020

Seems like the mapping part of json file needs to be wrapped into a "type" (which can be any name). This is on elasticsearch 6. I am not sure if older elastic used to accept without it.

e.g.

"mapping": {
  "asdasd": {
     "properties": { .... }
  }
}

@coveralls
Copy link

coveralls commented Jun 7, 2020

Coverage Status

Coverage decreased (-0.7%) to 86.634% when pulling 26f4172 on Carriyo:fix-mapping-template into 64c0bdd on vanthome:master.

@Munawwar
Copy link
Contributor Author

Munawwar commented Jun 7, 2020

Ok. I've changed a couple of things

  1. There had to be two template.json files - one for elasticsearch version 7 & above, and another one for elasticsearch version 6 & below

  2. Which means there has to be a way to know which ES version is being used. I introduced elasticsearchVersion field. As I think making a request for a not-so-common case (i.e. ensureMappingTemplate = true + mappingTemplate = undefined combination) is a bit of an overkill.. what do you think?

  3. However for tests, to test without specifying the template json file like it is now, I had to make a call to ES to find out the version.

  4. There is a bug in how log writing starts before ensuring template mapping succeeds. So fixed that as well.

  5. Another blocking bug I found on my setup is that the health check code fails with error 408 when you have more than one node in your elasticsearch cluster. I guess because the health check is hard-coded to '1'. It should rather be '>=1'.

@Munawwar Munawwar force-pushed the fix-mapping-template branch from ee91b41 to 5b1981f Compare June 7, 2020 20:14
@vanthome
Copy link
Owner

vanthome commented Jun 7, 2020

can you please rebase it?

Munawwar added 2 commits June 8, 2020 10:19
…ons..

.. so that issues like these are more visible
…mit message.

1. fix regression to support ES 7 as well as ES 6-
2. health check should use >=1 node else it never resolves
3. wait for template creation to complete before starting buffering
@Munawwar Munawwar force-pushed the fix-mapping-template branch from 5b1981f to 558cfa2 Compare June 8, 2020 06:23
@Munawwar
Copy link
Contributor Author

Munawwar commented Jun 8, 2020

@vanthome Done.

@teyou
Copy link

teyou commented Jun 8, 2020

Pls help update index.d.ts as well as there are more options in ElasticsearchTransportOptions.

related : #134

@Munawwar
Copy link
Contributor Author

Munawwar commented Jun 8, 2020

Added it now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants