Review Board

beta

Add search

Updated 12 months ago

David Trowbridge Reviewers
trunk reviewboard
216
None Review Board SVN 
This is an initial stab at fulltext and field-specific search.  It uses PyLucene/JCC,
which calls into the apache lucene implementation through the JNI.  The indexing is
done via a manage.py command "index" which can do either a full or incremental index
of the database.

There's still a few things left before this is perfect.  I need to figure out:
1. Review indexing.  I'm blocked on the fact that I can't import reviews now due
   to that bug that's being discussed on the list.
2. Filename indexing needs a better tokenizer.  I'll probably have to tokenize it
   myself.
3. It doesn't index dates yet.
4. This needs to be documented a lot -- especially how to build PyLucene, which is
   a total pain.
Used it a lot!  It's sweet!
Christian Hammond
Posted 1 year ago (January 3rd, 2008, 2:53 p.m.)

   

  

   
    
    
   




   



    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
Good fix though separate change :) I encountered this too.. I think we should probably just be setting reviewboard:url to a path without the http://, but we should protect against it here anyway.

     

  

   
    
    
   



    

    

    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
Is this part of this change? Seems unrelated and unfinished.

     

  

   
    
    

   
  1. 
    
    
     
    
      
      David Trowbridge
      1 year ago (January 3rd, 2008, 9:21 p.m.)
      
     
    
     
    Nope!  I've removed it.
    
    
    
   
    2. 


   



    

    

    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
Can you do str(request.id) ?

     

  

   
    
    
   



    

    

    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
str(request.id)

     

  

   
    
    
   



    

    

    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
I don't know Lucene. What's the difference between doing this and adding the blob at the end?

     

  

   
    
    

   
  1. 
    
    
     
    
      
      David Trowbridge
      1 year ago (January 3rd, 2008, 9:26 p.m.)
      
     
    
     
    The blob at the end is the default field.  This line specifically lets people search the summary field.
    
    
    
   
    2. 


   



    

    

    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
Not at all important, but it could be faster to request.bugs_closed.replace(",", " ")

     

  

   
    
    

   
  1. 
    
    
     
    
      
      David Trowbridge
      1 year ago (January 3rd, 2008, 9:28 p.m.)
      
     
    
     
    I'm gonna leave it as-is, because it's two lines otherwise.
    
    
    
   
    2. 


   



    

    

    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
What happens if get_full_name doesn't return anything but an empty string? Will it get mad if there's nothing there?

     

  

   
    
    

   
  1. 
    
    
     
    
      
      David Trowbridge
      1 year ago (January 3rd, 2008, 9:27 p.m.)
      
     
    
     
    Nope.  This string will get tokenized later by lucene.
    
    
    
   
    2. 


   



    

    

    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
This function is so generic (aside from the "ReviewRequest") that I think this could be in its own app somewhere, maybe even in djblets. The search function could take an initial queryset so that it knows the object type, and then filter off that.

I'd sure love to use this in other projects :)

     

  

   
    
    

   
  1. 
    
    
     
    
      
      David Trowbridge
      1 year ago (January 3rd, 2008, 9:30 p.m.)
      
     
    
     
    I'll leave it here for now, and figure out how to make this easy to integrate into other projects later.
    
    
    
   
    2. 


   



    

    

    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
result_ids = [int(lucene.Hit.cast_(hit).getDocument().get('id')
              for hit in hits]

? :)

     

  

   
    
    

   
  1. 
    
    
     
    
      
      David Trowbridge
      1 year ago (January 3rd, 2008, 9:52 p.m.)
      
     
    
     
    Done.
    
    
    
   
    2. 


   



    

    

    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
Put a space before the /. Some browsers get upset otherwise.

     

  

   
    
    

   
  1. 
    
    
     
    
      
      David Trowbridge
      1 year ago (January 3rd, 2008, 9:53 p.m.)
      
     
    
     
    Done.  I wouldn't mind someone giving this some CSS love ;)
    
    
    
   
    2. 


   



    

    

    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
Not sure we should assume there's a full name. In one of the other templates, we call something that gives us the proper display name (full name if possible, falling back on username).

     

  

   
    
    

   
  1. 
    
    
     
    
      
      David Trowbridge
      1 year ago (January 3rd, 2008, 9:54 p.m.)
      
     
    
     
    Fixed.
    
    
    
   
    2. 


   



    

    

   


  
 




  

 


















 

  




 

 

  
Ship it!

  
Christian Hammond

  
Posted 1 year ago (January 6th, 2008, 5:52 p.m.)

 

 

   
Looks good in general. One small thing (also, the post-review addition is still in there).

   

  

   
    
    

   
  1. 
    
    
     
    
      
      David Trowbridge
      1 year ago (January 7th, 2008, 12:36 a.m.)
      
     
    
     
    I've just committed the post-review change separately.
    
    
    
   
    2. 


   




   



    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
I'm going to be adding djblets_utils soon and moving things into that. This looks like a good candidate.

     

  

   
    
    

   
  1. 
    
    
     
    
      
      David Trowbridge
      1 year ago (January 7th, 2008, 12:33 a.m.)
      
     
    
     
    I'm actually thinking of making a context processor which would let people do {% if settings.key %} without having to include it in every view.
    
    
    
   
    2. 


   



    

    

    

     
     

      

       
       Loading diff fragment...
      

     

    

    

     
Seems this would be better as getattr(settings, setting, False)

     

  

   
    
    

   
  1. 
    
    
     
    
      
      David Trowbridge
      1 year ago (January 7th, 2008, 12:35 a.m.)
      
     
    
     
    Done.
    
    
    
   
    2.