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

Test using Docker based Solr rather than solr_wrapper #2012

Merged
merged 1 commit into from
Oct 10, 2019
Merged

Conversation

jcoyne
Copy link
Member

@jcoyne jcoyne commented Nov 1, 2018

No description provided.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 94.725% when pulling d57cd9f on docker-solr into a0c6247 on master.

@mejackreed
Copy link
Contributor

A question about this approach, do we need to offer a way that Blacklight adopters can easily run Blacklight + Solr locally for their development?

@jcoyne
Copy link
Member Author

jcoyne commented Nov 1, 2018

@mejackreed I think this still preserves generating the solr_wrapper into the host application gemfile, it just doesn't use solr_wrapper for rake ci

@dkinzer
Copy link
Member

dkinzer commented Nov 1, 2018

I think it will be easier on adopters to use docker compose. And running our application can be done using docker-compose up ... also, we can update rake blacklight:server to use docker-compose up for backwards compatibility.

Here is an example compose file that I've been using successfully for the last year: https://github.com/tulibraries/tul_cob/blob/master/docker-compose.yml

Note that I prefer not to bind the ports so that I can load multiple applications at once. But this is not necessary. Also I've configured to be able to work with traefik with which is not something most users will care about.

@jrochkind
Copy link
Member

Where does docker pull solr:7 get things from? Sorry, I'm not too familiar with docker.

This is getting a solr to run from... an image maintained by someone else? How does it have the schema that BL expects, or does BL no longer expect a non-default solr schema?

If new versions of solr are released, will this automatically get them? (Present solution does, theoretically. I am not at all opposed to giving that up though).

@jcoyne
Copy link
Member Author

jcoyne commented Nov 1, 2018

@jrochkind The image is pulled from the repository here: https://hub.docker.com/_/solr/
The 7 tag is used for the latest packaged version of solr 7.x.x. You could specify a more restrictive match if needed.

The config is sent to solr with the command here:
https://github.com/projectblacklight/blacklight/pull/2012/files#diff-354f30a63fb0907d4ad57269548329e3R32

@jrochkind
Copy link
Member

Ah, I see, thanks.

The present solution does that checksum-checking thing, which is an area that has lately been full of bugs/churn. This solution does not do that at all (which I think is fine), which avoids that whole thing being a problem. (Present solution could also have checksum-checking turned off).

@cdmo
Copy link
Member

cdmo commented Nov 2, 2018

FWIW, as a result of this conversation I decided to test out this docker solr approach for our CI and it worked great. We were having some issues getting solr_wrapper to run, and this worked pretty much right away. So, for CI, this seems like the easier option (speaking as a person fairly new to BL).

@mejackreed
Copy link
Contributor

Great @cdmo ! One more quick question, is there a way that running rake or rake ci from Blacklight can be setup so that this will all work correctly?

I'm trying to understand the CI vs local development modes here and how they might be used.

@dkinzer
Copy link
Member

dkinzer commented Nov 2, 2018

It might be that docker is just available and running in the travis environment. But locally a user would either have to have it running or enable it before running it. Someone in slack pointed this out as a possible hurdle to adoption; and I guess I agree.

@cbeer
Copy link
Member

cbeer commented Nov 2, 2018

👏

It'd be nice to make it easy to run tests locally that isn't:

In a separate terminal window, run:
$ docker pull solr:7
$ docker run -d -p 8983:8983 -v $PWD/lib/generators/blacklight/templates/solr/conf:/myconfig solr:7 solr-create -c blacklight-core -d /myconfig

I wonder if we ought to wrap that in a rake task? (Actually, saying that, I wonder if we could add a flag to solr_wrapper to have it use docker instead of downloading and running solr from source? Maybe that'd address some of the hurdles to adoption?)

@cdmo
Copy link
Member

cdmo commented Nov 5, 2018

@cbeer I was thinking the same thing and just made a quick attempt here. But, it actually works pretty well, including the :clean task. It needs to be DRYed out and abstracted out a bit. A full on docker-compose solution like what Temple has done with tul_cob is, for sure, more sophisticated. Oh, and we have a Procfile that calls this along with rails s and a command to run the webpacker server, so it's only one separate terminal window, which we like. Someone at the blacklight summit put us on Foreman, which we've found very useful.

@justinlittman
Copy link

Adding a +1 to this ticket. As newcomer to Blacklight, I show up understanding Docker (and Solr in Docker) but having to learn solr_wrapper.

@bess
Copy link
Member

bess commented Oct 10, 2019

Approved by group consensus at blacklight-summit 2019

@bess bess merged commit 1bfd473 into master Oct 10, 2019
@bess bess deleted the docker-solr branch October 10, 2019 19:38
@cbeer
Copy link
Member

cbeer commented Oct 10, 2019

It seems like we need to add some documentation about how to run the ci task locally? @jcoyne ?

mejackreed added a commit that referenced this pull request Mar 9, 2020
Note: automatic revert not possible.

Solr_wrapper was never fully removed and the docker based solr solution
was never documented. This PR restores the solr_wrapper approach for CI.

Fixes #2195
Fixes #2183
Fixes #2231
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.

None yet

9 participants