launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

The proposal to merge lp:~sinzui/launchpad/dsp-historic-attributes into lp:launchpad has been updated.

Description changed to:

Update the DSP vocab to support official packages and historic values.

    Launchpad bug: https://bugs.launchpad.net/bugs/826516
    Pre-implementation: no one

The DSP vocab was intended to solve a number of timeout and usability bugs
in target pickers, but it was flawed. Firstly it could take 20 minutes to
get return the results where we expect the query to be subsecond. The
vocabulary used soyuz publishing data, which meant that it did not support
official source package branches. The results were not ranked either, so
there was no guarantee that the exact match appeared on the first page of
results.

The DSPCache table used for archive searches contained much of the
information that was discussed as data needed for the DSP to make it
fast for searching official packages. A spike in SQL showed that a fast query
could be constructed to support official packages that only have branches
and the results could be ranked.

There were also concerns that the DSP vocabulary did not support historic
DSP values, those that were once valid, but users should not be able to
select them when reporting new bugs or asking questions.

This feature is behind the disclosure.dsp_picker.enabled flag and will only be
visible on qastaging. The UI needs love and the vocab itself needs to
prove it works with production data. Once we are certain of the data
that the DSP table must provide, we can fix the schema (bug 851266)
This branch implcitly fixes two other bugs about search suggests invalid
packages or does not match binary packages.

--------------------------------------------------------------------

RULES

    * Update the DSP vocab to permit the current value for a DSP so that
      pages with deleted packages will continue to work.
      * Used the AnswersDistributionVocab as a guide where the DSP used
        to initialise the vocab is always included in the vocab in the
        __contain__() and term rules existing values can be rendered in
        pages.
    * Replace the DSP search query with the spike query.
      * Limit the results to 60 (10 batches that the user might be willing
        to view)
      * Rank only the results that will be returned.
        * Exact match on source package name is first
        * Followed by exact match on binary package name
        * Then partial matches on the source package name
        * And lastly partial matches on the binary package name
      * Ensure that official DSPs that only have branches can be searched.
      * Ensure that only main and partner archives are searched.

QA

    To verify that the vocab, enable the feature:
    disclosure.dsp_picker.enabled	default	0	on

    * Visit https://bugs.qastaging.launchpad.net/charm/+source/apache2/+filebug
    * Verify  you cannot report a bug because apache2 does not exist.
    * Visit https://bugs.qastaging.launchpad.net/charm/+source/jenkins/+filebug
    * Verify you can report a bug.
    * Expand the task row to reveal the target widget.
    * Search for 'jenkins' using the package field's choose link.
    * Verify 'jenkins' is the first item in the listing
    * Search for 'slave'
    * Verify that 'jenkins-slave' is found
    * Search for 'ganglia-webfrontend'
    * Verify that is the first item in the listing.
    * Search for 'webfrontend'
    * Verify that is the listing.
    * Search for 'apache'
    * Verify there are no results
    * Visit https://bugs.qastaging.launchpad.net/ubuntu/+source/devicekit/+bug/695362
    * Verify the page loads.
    * Expand the bugtask
    * Enter any text into the comment field.
    * Save changes
    * Verify the page reloads without error
    * Verify the comment was added.

    Looking at a dev instance also requires the feature flag to be enabled.
    Search requires an update to the sample db because the data is incomplete.
        Insert into distributionsourcepackage(sourcepackagename, distribution)
        select sourcepackagename, distribution
        from distributionsourcepackagecache;

LINT

    lib/lp/registry/configure.zcml
    lib/lp/registry/interfaces/distributionsourcepackage.py
    lib/lp/registry/model/distributionsourcepackage.py
    lib/lp/registry/tests/test_distributionsourcepackage.py
    lib/lp/registry/tests/test_dsp_vocabularies.py
    lib/lp/registry/vocabularies.py

TEST

    ./bin/test -vvc -t TestDistributionSourcePackage.test \
        lp.registry.tests.test_distributionsourcepackage
    ./bin/test -vvc lp.registry.tests.test_dsp_vocabularies

IMPLEMENTATION

Added is_official so that callsites do not need to access the private
property. This property will probably change when we choose to exclude
delete packages from unsupported series.
    lib/lp/registry/configure.zcml
    lib/lp/registry/interfaces/distributionsourcepackage.py
    lib/lp/registry/model/distributionsourcepackage.py

Add a rule to record the DSP used as the context of the field and to
ensure it is always in the vocabulary. This ensures that historic packages
that have been deleted continue to work This also prevents errors when
users view bugs and questions targeted to packages that are now considered
invalid, but were not a few years ago.

Rework the search query based on a spike that uses the DSPCache table that
contains the words that the old query struggled to locate. The query supports
package branches unlike its predecessor and it is much faster. The UI is a
mess and I expect to make more changes to the vocab to fix the UI.
    lib/lp/registry/tests/test_distributionsourcepackage.py
    lib/lp/registry/tests/test_dsp_vocabularies.py
    lib/lp/registry/vocabularies.py

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/dsp-historic-attributes/+merge/78106
-- 
https://code.launchpad.net/~sinzui/launchpad/dsp-historic-attributes/+merge/78106
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/dsp-historic-attributes into lp:launchpad.