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.

Sign up for GitHub

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

Jump to bottom

bump elasticsearch version #4303

Merged
merged 2 commits into from
Apr 30, 2019
Merged

bump elasticsearch version #4303

merged 2 commits into from
Apr 30, 2019

Conversation

tasawernawaz
Copy link

What are the relevant tickets?

Fixes #3748

What's this PR do?

Bump elastic search version to 6.

How should this be manually tested?

Search should work in the app.

@odlbot odlbot temporarily deployed to micromasters-ci-pr-4303 April 22, 2019 11:50 Inactive
@odlbot odlbot temporarily deployed to micromasters-ci-pr-4303 April 22, 2019 13:26 Inactive
@odlbot odlbot temporarily deployed to micromasters-ci-pr-4303 April 22, 2019 14:04 Inactive
@tasawernawaz tasawernawaz marked this pull request as ready for review April 22, 2019 14:06
@odlbot odlbot temporarily deployed to micromasters-ci-pr-4303 April 22, 2019 14:11 Inactive
@tasawernawaz tasawernawaz force-pushed the tasawer/update-elastic-search-version branch from 1488b0b to 326dcde Compare April 23, 2019 10:18
@odlbot odlbot temporarily deployed to micromasters-ci-pr-4303 April 23, 2019 10:18 Inactive
@tasawernawaz tasawernawaz force-pushed the tasawer/update-elastic-search-version branch from 326dcde to c8a19ce Compare April 23, 2019 10:46
@codecov-io
Copy link

codecov-io commented Apr 23, 2019 •
edited
Loading

Codecov Report

Merging #4303 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4303      +/-   ##
==========================================
+ Coverage    94.9%   94.91%   +<.01%     
==========================================
  Files         501      501              
  Lines       22689    22692       +3     
  Branches      950      950              
==========================================
+ Hits        21534    21537       +3     
  Misses       1061     1061              
  Partials       94       94
Impacted Files Coverage Δ
search/api.py 96.52% <100%> (+0.02%) ⬆️
search/indexing_api.py 96.44% <100%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 307ad52...8886a8d. Read the comment docs.

@odlbot odlbot temporarily deployed to micromasters-ci-pr-4303 April 23, 2019 10:46 Inactive
@tasawernawaz
Copy link
Author

tasawernawaz commented Apr 23, 2019 •
edited
Loading

@noisecapella After bumping up elasticsearch version, few tests were failing. I have tried to fix them, still couple of tests are failing. Could you please look at those and review other changes.

It looks like percolate is deprecated now. elastic/elasticsearch#22331

@odlbot odlbot temporarily deployed to micromasters-ci-pr-4303 April 24, 2019 10:31 Inactive
@tasawernawaz tasawernawaz force-pushed the tasawer/update-elastic-search-version branch from c31fea3 to 5e2fd6f Compare April 24, 2019 11:12
@odlbot odlbot temporarily deployed to micromasters-ci-pr-4303 April 24, 2019 11:12 Inactive
@tasawernawaz tasawernawaz force-pushed the tasawer/update-elastic-search-version branch from 5e2fd6f to 8940ee6 Compare April 24, 2019 11:15
@tasawernawaz
Copy link
Author

@noisecapella I have used search instead of percolate method as they have removed it in the favor of search. Details at https://github.com/elastic/elasticsearch/pull/22331/files#diff-9c538c3ce0976b32ffd81a4ffeb389e4R6

Could you please do a review as build has passed now.

@mbertrand mbertrand self-assigned this Apr 29, 2019
mbertrand
mbertrand approved these changes Apr 29, 2019
View reviewed changes
Copy link
Member

@mbertrand mbertrand left a comment

Choose a reason for hiding this comment

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

Works great, just one minor suggestion.

search/indexing_api.py Outdated
@@ -230,6 +231,8 @@
}
}

TEST_INDEX_WILDCARD = '{index_name}_*'.format(index_name=settings.ELASTICSEARCH_INDEX)
Copy link
Member

Choose a reason for hiding this comment

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

This is also used in recreate_index, not just in unit tests, so maybe rename to INDEX_WILDCARD? Not 100% sure but maybe a value _all would work here too.

Copy link
Author

Choose a reason for hiding this comment

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

Done.
_all we override sometimes settings.ELASTICSEARCH_INDEX in tests, so don't want to risk it delete all indices.

@tasawernawaz tasawernawaz force-pushed the tasawer/update-elastic-search-version branch from 8940ee6 to 8886a8d Compare April 30, 2019 08:01
@tasawernawaz tasawernawaz merged commit e890c10 into master Apr 30, 2019
@tasawernawaz tasawernawaz deleted the tasawer/update-elastic-search-version branch April 30, 2019 10:10
@tasawernawaz
Copy link
Author

@pdpinch @mitodl/devops is there any pre/post requisite to the release of this upgrade ?

@blarghmatey
Copy link
Member

Yes, we will need to upgrade the ES instances, so we will coordinate with the release master for this.

rhysyngsun added a commit that referenced this pull request May 7, 2019
@rhysyngsun
Revert "bump elasticsearch version (#4303)"
1213532 
This reverts commit e890c10, reversing
changes made to 307ad52.
rhysyngsun added a commit that referenced this pull request May 7, 2019
@rhysyngsun
Revert "bump elasticsearch version (#4303)"
06f39f4 
This reverts commit e890c10, reversing
changes made to 307ad52.
@odlbot odlbot mentioned this pull request May 7, 2019
Merged
3 tasks
@markbreedlove markbreedlove mentioned this pull request Sep 13, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbertrand mbertrand mbertrand approved these changes

@noisecapella noisecapella Awaiting requested review from noisecapella

Assignees

@mbertrand mbertrand

Labels
Waiting on Author
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upgrade to elasticsearch 6
5 participants
@tasawernawaz @codecov-io @blarghmatey @mbertrand @odlbot