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

Search broken on Docker build #324

Closed
gasman opened this issue Mar 4, 2022 · 8 comments
Closed

Search broken on Docker build #324

gasman opened this issue Mar 4, 2022 · 8 comments

Comments

@gasman
Copy link
Contributor

gasman commented Mar 4, 2022

As per #319 (comment) and #323 (comment) - the Docker image is configured to use Elasticsearch, and it appears that this has been broken for some time prior to the upgrade to Wagtail 2.16. However, the addition of the update_index step to the load_initial_data command (to accommodate non-Docker setups which use the new database backend) now makes this breakage visible in the form of a "Connection refused" error during the data load.

Since the new database backend provides all the functionality we need for the demo, we should probably just drop Elasticsearch from the Docker config.

@vossisboss
Copy link
Contributor

I removed the Elasticsearch dependencies and references from the Docker compose file in #326. There is still an error related to a missing .env file but the bakery appears to run fine when these pieces are removed. I don't know the pros or cons of why Elasticsearch was added in the first place, but the update I made in #326 at least will get Docker users up and running without Elasticsearch breaking.

@vossisboss
Copy link
Contributor

@gasman I'll keep working on removing other references to Elasticsearch per our conversation at sprints today. One question though: It looks like the AWS packages (botocore and aws-requests-auth) as well as django-redis and django_cache_url were mostly added to support Elasticsearch. I think we should keep the AWS settings to give people a relatively easy way to add the demo to AWS, but I'm less sure about django-redis and django_cache_url. Will those be necessary without Elasticsearch?

@gasman
Copy link
Contributor Author

gasman commented Mar 30, 2022

Fixed in #326.

@gasman gasman closed this as completed Mar 30, 2022
@jsma
Copy link
Contributor

jsma commented Apr 8, 2022

Hi all,

Sorry to comment on a closed issue but I was about to start work on https://github.com/wagtail/docker-wagtail-develop which has this open todo to add elastic (the README also says "TODO: Set up an elasticsearch service container").

I setup the docker-wagtail-develop project and noticed backerydemo installed an old version of elasticsearch and came here only to see the most recent change was to remove elastic ;)

Would you welcome a PR to add modern Elastic stack support to this project? Or is the decision to deliberately keep this project simple with fewer dependencies?

Thanks!

@gasman
Copy link
Contributor Author

gasman commented Apr 8, 2022

@jsma I'd say there's a legitimate case for docker-wagtail-develop to come with an elasticsearch installation so that people can use it to develop and test against that backend, but it shouldn't be a dependency of bakerydemo itself. That's the approach taken by vagrant-wagtail-develop.

@jsma
Copy link
Contributor

jsma commented Apr 8, 2022

That makes sense. So would the hypothetical PRs then be:

bakerydemo

  • bump the current elasticsearch==5.5.3 to elasticsearch==7.17.2 or elasticsearch==8.1.2

vagrant-wagtail-develop

  • update to the latest 7.x or 8.x release of Elasticsearch server

docker-wagtail-develop

  • add the latest 7.x or 8.x release of Elasticsearch server

I haven't personally had a chance to work v8 just yet since we use enterprise-search with custom indexing logic (originally built before Wagtail had more robust site search features) but at the moment there is only an alpha release of enterprise-search-python that is v8 compatible.

However, we do have a solid docker-compose setup for Elastic stack v7.17.2 (elasticsearch, kibana, and apm-server). The official Elastic guides for using docker-compose have improved drastically in recent months so its much easier to get a secure stack up and running without much trouble. It's still a resource hog and slow to start of course so I completely understand not making it a hard dependency here.

@gasman
Copy link
Contributor Author

gasman commented Apr 8, 2022

bakerydemo

  • bump the current elasticsearch==5.5.3 to elasticsearch==7.17.2 or elasticsearch==8.1.2

No, any remaining references to elasticsearch in bakerydemo should be removed, as per #326 (comment).

vagrant-wagtail-develop

  • update to the latest 7.x or 8.x release of Elasticsearch server

docker-wagtail-develop

  • add the latest 7.x or 8.x release of Elasticsearch server

Yep. We don't currently support 8.x, and I don't know if anyone's looked into how much work it would be to support. It's also worth noting that Wagtail's support for Elasticsearch 6.x and above is sub-optimal, since they changed the way boosting works (wagtail/wagtail#5422) - however, that's probably all the more reason to upgrade our dev environments to it, so that people can work on fixing that :-)

@jsma
Copy link
Contributor

jsma commented Apr 8, 2022

I see. Per above, we don't use Wagtail's built-in search features at the moment so I wasn't thinking clearly earlier. Note to self: Wagtail works directly with elasticsearch and does not use enterprise-search.

In our use of enterprise-search, all boosting (aka "Relevance Tuning"), synonyms, "Curations", etc are handled in the enterprise-search dashboard by non-technical staff e.g. the marketing team. Relevance Tuning can be done through the API although the term used is "weights". We avoid doing this in code though since again, this is the marketing team's job and they shouldn't be bottlenecked on developers.

If the goal is for the Elasticsearch backends to continue using elasticsearch directly, I may not be the right person for the job given my lack of experience with this approach. If you're open to to a potential Elasticsearch8SearchBackend that breaks with the past to leverage enterprise-search (specifically the App Search component), I can perhaps start there although obviously I need to start by reading up on Wagtail's current search functionality ;)

No, any remaining references to elasticsearch in bakerydemo should be removed, as per #326 (comment).

Currently the vagrant|docker-wagtail-develop projects do a simple checkout of bakerydemo and elasticsearch is only installed through bakerydemo's own requirements, so the *-wagtail-develop projects would need to inject extra requirements, e.g. changing this:

# Install the bakerydemo project's dependencies into the image.
COPY ./bakerydemo/requirements/* /code/requirements/
RUN pip install --upgrade pip \
    && pip install -r /code/requirements/production.txt

To this (where each project provides it's own requirements/elastic.txt to be copied in before installing):

# Install the bakerydemo project's dependencies into the image.
COPY ./bakerydemo/requirements/* /code/requirements/
COPY ./requirements/elastic.txt /code/requirements/
RUN pip install --upgrade pip \
    && pip install -r /code/requirements/production.txt -r elastic.txt

Is that the idea?

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

No branches or pull requests

3 participants