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

Added information related to viaq_index_name field #115

Merged
merged 1 commit into from
Nov 13, 2019

Conversation

mijicherian
Copy link

No description provided.

@richm
Copy link
Member

richm commented Oct 31, 2019

@lukas-vlcek any idea why the test failed?

@lukas-vlcek
Copy link
Member

lukas-vlcek commented Nov 1, 2019

It fails because you are introducing a new field viaq_index_name. When the test "wget"s released index template (from Github) and compares it with newly generated index template it does not find the field viaq_index_name in released index template. It is found only in the newly generated index template. And this is considered an issue (diff is not empty).

You can navigate to failed test details and search for viaq_index_name string. It will give you some clue about this.

Anyway, in cases like this it is needed to introduce an exception for this field into the test and remove this field from generated model. This means removing relevant part of JSON document.

How to do it:

The line where the new (fresh) index templated is generated in the test is here. Right after this the generated model is modified depending on which released model we want to compare it with (i.e. target ES version). Given, this field has been introduced just now I think we will need to remove this field for all target ES versions (the field is not part of any released model at this moment).

So I would recommend adding a new method to remove relevant mapping like this and call it in the end of each if-then blocks: here, here and here.

I can do it if you want.

Later, when we decide which model versions this new field will be propagated to and after relevant releases are made we will remove the exception from test for those versions.

@lukas-vlcek
Copy link
Member

Also something similar will have to be fixed for index patterns test somewhere here.

But let's fix the index template test first.

lukas-vlcek added a commit that referenced this pull request Nov 13, 2019
lukas-vlcek added a commit that referenced this pull request Nov 13, 2019
@richm richm merged commit 24805e0 into ViaQ:master Nov 13, 2019
@mijicherian
Copy link
Author

Can you please let me know the Licencing details of this repository?

@lukas-vlcek
Copy link
Member

@mijicherian Good point, seems we forgot to add LICENSE details into this repo. I am sure it should have been AL2 but let me check if we can add it retrospectively.

@richm
Copy link
Member

richm commented Nov 14, 2019

Yes, we can add it retroactively

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.

3 participants