Revert "Test using Docker based Solr rather than solr_wrapper"

[mejackreed]

This reverts commit 0759cd5.

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

While I acknowledge that this maybe in fact controversial, I would like to propose this move forward since we didn't seem to have documentation and local ci traction with the other approach. I'm also open to solutions that resolve the documentation and local development gaps.

Fixes #2195
Fixes #2183
Fixes #2231

[mejackreed]

I would like to leave this PR open for a few days to gather feedback from others, since I recognize it might be controversial.

[jcoyne]

Solr_wrapper isn't maintained. I've put in PRs over 18 months ago that haven't been merged or received a review: cbeer/solr_wrapper#119

Furthermore, solr_wrapper doesn't manage the version of Java you need to run Solr (see #2216 (comment))

[mjgiarlo]

What would need to happen to keep the docker-based configuration in? Better documentation and CI integration?

[jcoyne]

we already are using docker in ci: https://github.com/projectblacklight/blacklight/blob/master/.travis.yml#L29

[mejackreed]

Right now, a developer looking to run the tests for Blacklight cannot without undocumented intervention steps. Our ci is reliant on travis configs (see #2231). There are large drifts in documentation for the docker approach. First time, newer, or even long time contributors are having difficulty with running the tests locally.

I'm 💯 open to other solutions, but with issues continuing to be created about this issue, and the documentation not being resolved, I have concerns that the docker approach is viable with enough maintainable interest.

[jcoyne]

@mejackreed a lot of the documentation we have even pre-dates solr_wrapper: https://github.com/projectblacklight/blacklight/wiki/README_SOLR#getting-solr. So, I think we need to specify which documentation should be kept and which should we should remove.

We had a conversation about using docker at the last Blacklight meeting and the room was very supportive of moving to docker from solr_wrapper.

[camillevilla]

I'm agnostic to approach, but I would appreciate it if other folks who are in favor of the Docker-based approach weighed in on #2186. @jcoyne and @dkinzer seem to disagree on approach there. If there are additional Dockerheads in the community, their feedback would be more useful there instead of in this PR.

[jcoyne]

@camillevilla I think #2186 is a bit orthagonal, as that's about running the development rails server in Docker, not just running solr in docker.

[mjgiarlo]

@jcoyne 💬

@camillevilla I think #2186 is a bit orthagonal, as that's about running the development rails server in Docker, not just running solr in docker.

Not totally orthogonal, though, right? Because it introduces the docker-compose.yml config that could serve as a cornerstone of making testing easier and clearer for everyone.

[dkinzer]

Hey all, I've updated #2186 to address @jcoyne's concerns. Hopefully that can move forward now. I agree with @jcoyne that we need to move away from solr_wrapper.

[aaron-collier]

I'm curious why this approach is preferred over completing the docker based approach and getting it properly documented? Especially considering the lack of solr_wrapper maintenance.

[jkeck]

I've left #2186 (comment), but IMO, the approach that is accurately reflected in the documentation has value. I'm not opposed to going to docker, but I am opposed to dramatically updating our development setup w/o properly documenting it (which is what has already happened).

[jcoyne]

@jkeck This changes nothing but how you start solr to run a test. The only requirement we have is that you have solr started somewhere. You can do this with a SAAS or jettywrapper or solr_wrapper or just by using Java. As far as I can tell, there is nothing in our documentation that is currently broken (The quckstart guide explains how you can install solr_wrapper).

[jkeck]

Agreed, and changing how this was done would have ideally been documented appropriately along with the change. Even if the setup wasn't documented/the documentation was out of date, making drastic changes to that setup would benefit from being documented.

I understand that there was general support at the Blacklight summit for moving to a Docker based approach, but relying on those who were not in the room to figure it out and do that documenting hasn't panned out to date.

[jkeck]

I can't speak for everybody, from IMO, it seems like a possible way forward is documenting what's going on.

Thanks to @mjgiarlo for offering to do that in #2186 (comment).

[jcoyne]

I think this issue is very different from #2186 and we should consider them very separately. This issue is about how we start solr. #2186 is about where the development environment runs.

[mjgiarlo]

@jcoyne 💬

I think this issue is very different from #2186 and we should consider them very separately. This issue is about how we start solr. #2186 is about where the development environment runs.

docker-compose, introduced in #2186, has elsewhere been suggested as a useful post-solr_wrapper mechanism for spinning up solr (and other dependencies). That's what ties them together. I.e., I'd like to see #2186 merged soon so we can make progress on a clear set of documentation about how BL devs do dev and testing. Does that sound reasonable? cc: @jkeck

[mejackreed]

FWIW, I agree with with @jcoyne in the sense that #2186 adds both the Solr and Rails pieces together under docker. And that it would then have CI Solr still run differently than a local development approach.

There are at least 6 other Blacklight engines that I help maintain that are still using solr_wrapper. To that end I think a solution based on or extended from #2186 should be able to do the following:

• Specify Solr versions
• Enable other Blacklight engines to use a similar command
• Split Solr from Rails applications running
• Customize Solr configurations

This then makes me wonder if the comment here #2012 (comment) is an alternative viable approach? It seems to be 👍 by both @jcoyne and @dkinzer

[mjgiarlo]

@mejackreed 💬

There are at least 6 other Blacklight engines that I help maintain that are still using solr_wrapper. To that end I think a solution based on or extended from #2186 should be able to do the following:

• Specify Solr versions
• Enable other Blacklight engines to use a similar command
• Split Solr from Rails applications running
• Customize Solr configurations

This is a super helpful list. Thank you!

This then makes me wonder if the comment here #2012 (comment) is an alternative viable approach?

I'd be reluctant to do anything based on solr_wrapper without a clearer sense of how we might maintain that as a community. Personal gems as core testing/dev infrastructure seems like an anti-pattern to me.

[jkeck]

Agree that these issues are different, and unfortunately this conversation has split between them, but the crux of the issue is the same. If this change had been documented (as described above and requested 5 months ago) this PR would have never been submitted to revert this work (apologies if I’m mischaracterizing that).

I’m trying to avoid the same thing happening over there, hence my comment.

I’m not trying to hold any of this work up, simply advocating for responsibly documenting changes as we make them (which should not be controversial)

[jcoyne]

@jkeck what I'm saying is that reverting this PR is unnecessary, because we already have a documented way to start solr_wrapper in the quickstart.

[mjgiarlo]

For now, since I'm committing a few hours of my time this week to making improvements to the dev/test experience and documentation thereof, I'm going to temporarily block this PR to prevent accidental merges. I plan to revisit this PR by Friday afternoon. Thanks for the robust discussion, fellow Blacklighteers.

[cdmo]

Apologies for the lengths, TLDR: better docs would be 💯

I think there are two separate topics being discussed (at least anyway): (1) spinning up a new Rails app to develop on (like the Quick Start Guide describes) and then there's (2) "running core blacklight's internal test app to do some dev work on Blacklight itself."

For (1) as others and @jcoyne have suggested, I think the guide should be updated to use docker (see #2183). Maybe further discussion on that topic can happen there?

So to me this particular PR is more about (2) (and also about Travis because these two things are bound together). I guess I'm not in favor of reverting and at this point I'd actually prefer these not-perfect simple instructions for running the test suite locally. Docker is already working in Travis, and it's just locally a little cumbersome (let's fix that part).

I'll add, as someone who just started working in Blacklight a little over a year ago, solr_wrapper is pretty foreign to me, I've rarely used it and would prefer to not have to think about selfishly (even given the less-than-perfect dev contribution instructions). That said, if the community wanted to use it, then I'd be ok with that too.

[mjgiarlo]

@cdmo 💬

I think there are two separate topics being discussed (at least anyway): (1) spinning up a new Rails app to develop on (like the Quick Start Guide describes) and then there's (2) "running core blacklight's internal test app to do some dev work on Blacklight itself."

For (1) as others and @jcoyne have suggested, I think the guide should be updated to use docker (see #2183). Maybe further discussion on that topic can happen there?

Good suggestion: #2183 (comment)

So to me this particular PR is more about (2) (and also about Travis because these two things are bound together). I guess I'm not in favor of reverting and at this point I'd actually prefer these not-perfect simple instructions for running the test suite locally. Docker is already working in Travis, and it's just locally a little cumbersome (let's fix that part).

In my in-progress PR, I am amending the default rake task to spin up Solr via docker-compose, btw. What I am less clear on is where to document this. README? Contribution guide? New wiki page?

I'll add, as someone who just started working in Blacklight a little over a year ago, solr_wrapper is pretty foreign to me, I've rarely used it and would prefer to not have to think about selfishly (even given the less-than-perfect dev contribution instructions). That said, if the community wanted to use it, then I'd be ok with that too.

💯

[mjgiarlo]

@mejackreed @jcoyne @camillevilla @dkinzer @cdmo @aaron-collier @jkeck @jrochkind

I'd welcome your 👀 on #2263 (comment)

[mjgiarlo]

@mejackreed @jcoyne @camillevilla @dkinzer @cdmo @aaron-collier @jkeck @jrochkind

I'd welcome your on #2263 (comment)

#2263 is now 💚 and ready for reviews. It has been lovingly described and annotated to (hopefully) help reviewers. Please do note the big open question in the PR description. I would like to see a consensus answer on that one before merging the PR. Would ideally like to get additional feedback from folks who we haven't heard from yet: @mejackreed @camillevilla @aaron-collier @jkeck @jrochkind

[mejackreed]

Thanks @mjgiarlo will take a look at it today.