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

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

[Reyna, David]

[-- Attachment #1: Type: text/plain, Size: 2348 bytes --]

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).

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)?

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.

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.

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


[-- Attachment #2: Type: text/html, Size: 4117 bytes --]

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

[Barros Pena, Belen]

On 15/03/2014 07:05, "Reyna, David" <david.reyna@windriver.com> wrote:

>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.

This works for me. I can no longer reproduce the problem.


Cheers

Belén

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

[Damian, Alexandru]

[-- Attachment #1: Type: text/plain, Size: 3164 bytes --]

Comments inline,

Alex


On Sat, Mar 15, 2014 at 7:05 AM, Reyna, David <david.reyna@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

[-- Attachment #2: Type: text/html, Size: 6549 bytes --]