[Toaster] review request: Bug #5962 The "Local configuration variables" filter returns an error when applied on top of search results

[Damian, Alexandru]

Comments inline,

Alex


On Sat, Mar 15, 2014 at 7:05 AM, Reyna, David <david.reyna at windriver.com>wrote:

>  Hi Belen and Alex,
>
> It took a lot of trial and error, but I now have a solution for 5962 :
>
>     dreyna/regex_filter_5962
>
> Alex, I wanted to call out the nasty technical issues I encountered, plus
> the solution that I finally put into place. The reason for this is that I
> discovered that two methods that should have worked both failed, and I had
> to go to a more heuristic solution in the end to satisfy the requirements
> (which I am not totally happy about).
>
> 1) Original failure.
>
> It turns out that:
>
>   IF you select a filter that uses regex with alternation (e.g.
> 'vhistory__file_name__regex:conf/(local|bblayers).conf') …
>     AND you add a search string …
>     AND there are more than one ‘search_allowed_fields’ value …
>     THEN (and only then) you will get a Django database fatal error.
>
> This is nasty. The “views.py” will build the queryset, but it is damaged
> and the first lookup returns a “Exception Value: user-defined function
> raised exception“ (I am guessing it is against the lamda’s that filter the
> Q objects for ‘search_allowed_fields’ fields).
>

>
​[Alex] This seems to be a problem with SQLite3 and string encoding. Ufff,
this is a tough one.​



> 2) I then tried to use the facility to specify multiple filter compares
> that ” _get_filtering_query” would appear to support.
>
>
> ‘vhistory__file_name__icontains;vhistory__file_name__contains:conf/local.conf;conf/bblayers.conf'
>
> I however could not get both clauses to work. I even used ‘contains’ and
> then ‘icontains’ to get around the dictionary overwrite.
>
> Is this the expected usage? Is this feature supposed to work (I see no
> other examples in “views.py)?
>

[Alex] This is expected usage, and yes, it works. But the operation is
"AND" not "OR".​

>
> 3) My final solution was to do a simple file path test against
> “<project_path>/conf/”, with the observation that only the local
> configuration files meet this criteria. This seems a hack, but it works and
> I have no alternate proposal at this time.
>

[Alex] ​​I guess it's an ok hack.

>
> 4) By the way, I observed that the database schema does not seem to
> provide the path to the project’s directory. In #3 I cheated and munged the
> “cooker_log_path” value.
>

[Alex] Not very happy about this one. This value should be in ​​the
Variables table, see "THISDIR" variable that holds the local conf path. The
"/tmp/log" hardcoding seems a bit strange, what is the intended usage ?


> I suggest that providing the formal path in the database is very important
> to general users and especially build farm managers (who will also want
> access to the machine name and/or IP address), so that they can correlate a
> build report with its definitive location from their remote location.
>
> I think that I will file a defect to track this concern.
>
> Thanks,
> David
>
>



-- 
Alex Damian
Yocto Project
SSG / OTC
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.yoctoproject.org/pipermail/toaster/attachments/20140320/ccd40466/attachment-0001.html>