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

[BD-19] Transition to the Elasticsearch 7.8.0 version #104

Conversation

Golub-Sergey
Copy link
Contributor

@Golub-Sergey Golub-Sergey commented Aug 4, 2020

Description: transition from es1.5 to es7. Replaces index courseware_index with doc_types courseware_content and course_info to indexes courseware_content and course_info

Was done:

  • check code coverage and tests passing;
  • update code base;
  • update make file;
  • update travis ci.

Code coverage: 99%

Notes:

  • Changes to the edx-platform which apply edx-search updates

@openedx-webhooks
Copy link

openedx-webhooks commented Aug 4, 2020

Thanks for the pull request, @Golub-Sergey! I've created BLENDED-532 to keep track of it in Jira. More details are on the BD-19 project page.

When this pull request is ready, tag your edX technical lead.

@openedx-webhooks openedx-webhooks added blended PR is managed through 2U's blended developmnt program needs triage labels Aug 4, 2020
@stvstnfrd
Copy link

@Golub-Sergey I'm trying to run this locally for testing but having difficulty (CC: @dianakhuang).

Would you mind including a write-up of the steps you used to configure this to run in your devstack?

I think I'm running the right code/config everywhere (see below), but first, never got results for any queries and now, after some more manual fiddling, get errors.

I'm using this devstack branch [1] to run this version of search [2] via this version of platform [3].
Is there anything in particular that jumps our re:misconfiguration?
I've tried relying on publish-based index, manually re-indexed, used existing courses, created new courses, but still no luck.

@Golub-Sergey
Copy link
Contributor Author

@Golub-Sergey I'm trying to run this locally for testing but having difficulty (CC: @dianakhuang).

Would you mind including a write-up of the steps you used to configure this to run in your devstack?

I think I'm running the right code/config everywhere (see below), but first, never got results for any queries and now, after some more manual fiddling, get errors.

I'm using this devstack branch [1] to run this version of search [2] via this version of platform [3].
Is there anything in particular that jumps our re:misconfiguration?
I've tried relying on publish-based index, manually re-indexed, used existing courses, created new courses, but still no luck.

Hello, @stvstnfrd . Can you please use this version of the edx-platform. I tested locally this changes and it works and searches correctly. This branch was forked from #104 branch. Current version of edx-search lib without platform changes produces errors.

@Golub-Sergey Golub-Sergey force-pushed the golub-sergey/BD-19/transition-from-ES1.5-to-ES7 branch from bec4646 to 6d218df Compare September 18, 2020 09:12
Copy link

@stvstnfrd stvstnfrd left a comment

Choose a reason for hiding this comment

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

@Golub-Sergey A few questions/suggestions, but I have this running successfully in my devstack now :)

Makefile Outdated Show resolved Hide resolved
search/views.py Show resolved Hide resolved
services:

test_elasticsearch:
image: elasticsearch:7.8.0

Choose a reason for hiding this comment

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

Hrmm, is there any way to (easily) keep this in sync w/ the value listed in the requirements file(s) below?
Otherwise, this will probably fall out of step.

I'm not sure how/if this is handled elsewhere in the ecosystem...

Not a deal-breaker, just curious...

search/api.py Show resolved Hide resolved
search/tests/test_views.py Show resolved Hide resolved
search/elastic.py Outdated Show resolved Hide resolved
@Golub-Sergey
Copy link
Contributor Author

@stvstnfrd Hello, I recreated pr to platfrom which applies edx-search lib changes to the edx-platform
https://github.com/edx/edx-platform/pull/25081
Can you, please, restart ci

@timmc-edx
Copy link
Contributor

Copy link

@stvstnfrd stvstnfrd left a comment

Choose a reason for hiding this comment

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

@Golub-Sergey Okay, this looks good to me. Before merge, we'll want to bump the version number in setup.py. Given these breaking changes, we should probably bump up to a 2.0.0 release. Thoughts?

@Golub-Sergey
Copy link
Contributor Author

@Golub-Sergey Okay, this looks good to me. Before merge, we'll want to bump the version number in setup.py. Given these breaking changes, we should probably bump up to a 2.0.0 release. Thoughts?

Thats a good idea. Done.

@dianakhuang dianakhuang merged commit 711e642 into openedx:master Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blended PR is managed through 2U's blended developmnt program merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants