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

Remove filetypes fq #2288

Closed
wants to merge 2 commits into from
Closed

Remove filetypes fq #2288

wants to merge 2 commits into from

Conversation

mccalluc
Copy link
Member

Fix #2106?

@mccalluc
Copy link
Member Author

3 fails, just because solr query doesn't match:

FAIL: test_generate_solr_params_for_user (user_files_manager.tests.UserFilesUtilsTests)
FAIL: test_generate_solr_params_for_assay_with_params (data_set_manager.tests.UtilitiesTest)
FAIL: test_generate_solr_params_no_params (data_set_manager.tests.UtilitiesTest)

I'll fix those.

@mccalluc
Copy link
Member Author

One of two builds fails:

======================================================================
FAIL: test_get_dataset_expecting_analyses (core.tests.DataSetResourceTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/refinery-platform/refinery-platform/refinery/core/tests.py", line 1991, in test_get_dataset_expecting_analyses
    self.assertEqual(data['analyses'][0]['name'], a2.name)
AssertionError: u'a1' != 'a2'

Haven't seen this before. Just try restarting...

@codecov-io
Copy link

Codecov Report

Merging #2288 into develop will increase coverage by 1.51%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2288      +/-   ##
===========================================
+ Coverage    50.37%   51.89%   +1.51%     
===========================================
  Files          377      377              
  Lines        26916    27765     +849     
  Branches      1241     1241              
===========================================
+ Hits         13560    14409     +849     
  Misses       13356    13356
Impacted Files Coverage Δ
refinery/data_set_manager/utils.py 76.1% <ø> (-0.05%) ⬇️
refinery/user_files_manager/tests.py 100% <ø> (ø) ⬆️
refinery/data_set_manager/tests.py 99.67% <ø> (ø) ⬆️
refinery/tool_manager/views.py 98.82% <0%> (-1.18%) ⬇️
refinery/tool_manager/tests.py 99.95% <0%> (+0.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20d41e2...db54bb0. Read the comment docs.

Copy link
Member

@scottx611x scottx611x left a comment

Choose a reason for hiding this comment

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

Was the consensus on this still that we weren't sure as to what the 0 Facet Value behavior in the Filebrowsers should be so theres no clear path to proceed?

@mccalluc
Copy link
Member Author

I think that's where we left it... There are a couple ideas out there:

  • Load up some data locally and see what it looks like with these changes.

but also:

  • Maybe if we think it through, we'll realize that no amount of solr tweaking will give us the results we want, and it may require changes to the indexing.

@mccalluc
Copy link
Member Author

mccalluc commented Nov 6, 2017

Thinking a little more: The simplest thing is probably just not to index these documents in the first place... but still store the files, in case we want to reindex and make them visible?

@mccalluc mccalluc closed this Nov 6, 2017
mccalluc added a commit that referenced this pull request Nov 6, 2017
@mccalluc mccalluc deleted the mccalluc/remove-filetype-fq branch November 9, 2017 15:04
scottx611x pushed a commit that referenced this pull request Jan 2, 2018
* Skip indexing for some files

* Apply changes from #2288: Bad docs will be excluded from index rather than excluded by query.

* 2nd thought, not config, but make it a Node class constant
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

3 participants