-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
In-progress Solr containerization and reindexing work #1176
Conversation
OK - Sebastian, my suspicion was correct -- this didn't pass, due to our new test, but it didn't trigger a failure for Travis. That's in https://github.com/jywarren/web/issues/70 |
@ananyo2012 - this check failed because we added new code that's not as thoroughly covered, it say, but I introduced one new controller action and wrote a test for it. Would I have to put that test in the conventional /test/functional/ as well (or rather than in /test/solr/, where I put it?) to get the check to pass? |
@ananyo2012 - just wondering about this failed check and if we need to either: a. make Coveralls aware of our solr tests Thanks! |
@jywarren I am not quite sure why the coverage decreased. Have to check how coveralls calculates the coverage. Maybe its because the solr tests weren't taken into account so far and now that they are being tested there is a significant drop in the coverage so we need more solr test. Or maybe it is due to less tests for drupal_node model as I see a drop in the coverage of drupal_node.rb |
Well, I did add a new method but only added a corresponding test in the
solr tests... which aren't a standard place to keep tests.
Where is the coveralls config for where it looks for tests?
Thanks!
|
Generated by 🚫 Danger |
Hi Jeff, |
Wow!!! Very cool. Checking now. |
I see a |
I guess so, I'm just looking at what to look for...
…On 14/02/17 12:46, Jeffrey Warren wrote:
I see a |That page does not exist.| -- a 404, for
http://branch1.laboratoriopublico.org/searches/test. That's odd, no?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1176 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAMMSycIxArd7GZLL-uPZuDAs9WtDcilks5rceiEgaJpZM4Lc_9v>.
|
There is something here:
http://branch1.laboratoriopublico.org/searches/2
I got there from:
http://branch1.laboratoriopublico.org/search
This is different from what I see in staging, a 500 error.
But it doesn't look right (results appear to be always the same).
Glad this is (finally) moving forward!
Regards.
Sebastian
…On 14/02/17 12:46, Jeffrey Warren wrote:
I see a |That page does not exist.| -- a 404, for
http://branch1.laboratoriopublico.org/searches/test. That's odd, no?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1176 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAMMSycIxArd7GZLL-uPZuDAs9WtDcilks5rceiEgaJpZM4Lc_9v>.
|
Indeed - i see:
for any search. I also noticed you have to be logged in to search, good to fix that too at some point. |
Currently we've managed to reindex, but the tests of searches don't work as they are expected - we get the same results over an over. Currently we need to figure out how the query to the Solr engine is being built, whether it is correct and the index is wrong, or the query itself is wrong. The route to the advanced search as is running in branch1 testing instance is at: |
I think we might try to add a test to the solr tests which asserts that the
raw index contents for two different records are not the same?
…On Fri, Feb 17, 2017 at 12:08 PM, Sebastian Silva ***@***.***> wrote:
Currently we've managed to reindex, but the tests of searches don't work
as they are expected - we get the same results over an over. Currently we
need to figure out how the query to the Solr engine is being built, whether
it is correct and the index is wrong, or the query itself is wrong.
The route to the advanced search as is running in branch1 testing instance
is at:
http://branch1.laboratoriopublico.org/searches/new
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1176 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJykTlj4l-bGjkqRmPqzzAmy5b9e7ks5rddQjgaJpZM4Lc_9v>
.
|
Attempting an advanced search, while not logged in, produces:
|
I'm documenting here what I find. When logged in, logs show:
Going to check Solr logs now. |
I searched for my name on users and Solr logged:
|
This was my query:
|
Well the index has +800k documents, but I'm getting the same 10 results for every search. |
This may be related to the issues that were fixed in #992 If redoubled my efforts and went hunting over the rails console. It is creating a search record, but is not saving the key_words: I tried searching over the advanced search form and then went to examine the resulting SearchRecord.
|
Indeed it gets sent from the form:
|
And here, doing it in the console: 2.1.2 :002 > s = SearchRecord.new({key_words: 'balloon'})
=> #<SearchRecord id: nil, key_words: "balloon", title: nil, user_id: nil, created_at: nil, updated_at: nil, main_type: nil, note_type: nil, created_by: nil, max_date: nil, language: nil, min_date: nil>
2.1.2 :003 > s.save
(0.5ms) begin transaction
SQL (51.0ms) INSERT INTO "searches" ("created_at", "created_by", "key_words", "language", "main_type", "max_date", "min_date", "note_type", "title", "updated_at", "user_id") VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) [["created_at", Fri, 24 Feb 2017 16:09:08 UTC +00:00], ["created_by", nil], ["key_words", "balloon"], ["language", nil], ["main_type", nil], ["max_date", nil], ["min_date", nil], ["note_type", nil], ["title", nil], ["updated_at", Fri, 24 Feb 2017 16:09:08 UTC +00:00], ["user_id", nil]]
(19.5ms) commit transaction
=> true
2.1.2 :005 > SearchRecord.find 1
SearchRecord Load (0.9ms) SELECT "searches".* FROM "searches" WHERE "searches"."id" = ? LIMIT 1 [["id", 1]]
=> #<SearchRecord id: 1, key_words: "balloon", title: nil, user_id: nil, created_at: "2017-02-24 16:09:08", updated_at: "2017-02-24 16:09:08", main_type: nil, note_type: nil, created_by: nil, max_date: nil, language: nil, min_date: nil> |
Okay, doing it manually like that in the rails console actually saves the |
Disabled a faulty test |
Adding a test to ensure that results of |
It reindexed but we're not running tests on this instance. I'll enable them on the jenkins config http://jenkins.laboratoriopublico.org/job/Plots-Solr/19/console |
I've changed the configuration and built manually http://jenkins.laboratoriopublico.org/job/Plots-Solr/20/console - let's see. |
Hi, |
This took 32 minutes and produced |
Hmm, so that test plausibly failed because there were no results -- nothing was indexed? I pushed another change just to give us a look at what's in the response, rather than letting the JSON parsing error out (which yields less helpful info). I think it's possible that since there are no results, lots of code isn't used (since some tests rely on getting responses). But I'm not sure about that. I'm also trying to get the line-by-line Coveralls analysis working by tweaking the Coveralls web UI settings, as mentioned in the comments on this post: https://coveralls.zendesk.com/hc/en-us/articles/201342479-Quick-help-For-Common-Problems |
Whoa okay there it is! Compare failing node.rb from last commit: https://coveralls.io/builds/11195716/source?filename=..%2Fmodels%2Fnode.rb To passing from current master branch: https://coveralls.io/builds/11195716/source?filename=..%2Fmodels%2Fnode.rb Oh I see -- it passed because coveralls isn't aware of multiple branches. So it's just saying its the same vs the last run on this branch. |
Okay, the Jenkins was unable to reindex because Solr is disabled in config Great that the coverage mystery and the reindexing issues are uncovered! |
But the fact that it didn't reindex or that it has Solr disabled, didnt affect its ability to launch: http://branch1.laboratoriopublico.org/ is up and working with Solr disabled, as desired. |
I've gone ahead and changed sunspot.yml directly in branch1 at tycho, and reindexing with |
OK, great -- the only reason I'm still waiting on this is to see why exactly coverage dropped -- i'm manually comparing the two line-by-line coveralls analyses of node.rb. Then we'll merge and let's just remember that the solr-specific tests may be failing but it's offline so that's OK... Note that these lines are no longer covered, for some reason:
(Next few lines revised) I also note that the whole thing runs in 9 minutes, with 163 unit tests (3 new ones): https://travis-ci.org/publiclab/plots2/builds/224713401 ...not the more typical 12 minutes of recent master branch runs, which actually have fewer unit tests -- 160 unit tests: https://travis-ci.org/publiclab/plots2/builds/224893593 Looking into this more... |
Allright, after 25 hours of reindexing, the indexer has all the info it seems. We will need to figure out how we can reindex in less time than that! |
|
OK - well, do you think the Solr-specific test that was failing (due to a null result) would pass now? Would re-triggering this Travis check restart/reset the indexing, though? |
Yes actually I think the tests should pass. Nothing is retained in Travis, if it has a Solr engine at db creation / setup time and the data is there, it should respond with results, AFAICT. |
Ah, yes -- so perhaps So now we're back where we were, which was just figuring out the coverage drop before merging this. |
795c57d
to
315fe76
Compare
Finally. I expect pushing this to master should not break staging instance. |
Merging this now. Next checklist should be:
Then, the rest of this:
|
OK, remaining checklist moved to #1386 |
@icarito and I are working on this branch to resolve Solr issues through better containerization; working on this in #1112.
This will show a pass/fail based on any commits pushed to this branch:
http://staging.laboratoriopublico.org/job/Plots-Solr/
http://branch1.laboratoriopublico.org/
Look for success here:
http://branch1.laboratoriopublico.org/searches/test