launchpad-reviewers team mailing list archive

[Ines Almeida <mp+493278@xxxxxxxxxxxxxxxxxx>]

Added a few comments
Can you add a test for updating these extra_attrs i.e. if there is already extra_attrs and we update the CVE?

Diff comments:

> diff --git a/lib/lp/bugs/scripts/soss/tests/sampledata/CVE-2011-5000 b/lib/lp/bugs/scripts/soss/tests/sampledata/CVE-2011-5000
> index 76ce151..97e27a9 100644
> --- a/lib/lp/bugs/scripts/soss/tests/sampledata/CVE-2011-5000
> +++ b/lib/lp/bugs/scripts/soss/tests/sampledata/CVE-2011-5000
> @@ -13,3 +13,5 @@ Packages:
>      Status: ignored
>      Note: ''
>  Candidate: CVE-2011-5000
> +Extra_attrs:

Nit: Have you/Can you check with the Allen Huang where this Extra_attrs would go in the CVE file to make sure we add it in the right place in our test data?

> +  Custom_attr_1_nvd_score_arrivial_time: '2012-09-09T25:21:39Z'
> diff --git a/lib/lp/bugs/scripts/soss/tests/test_sossimport.py b/lib/lp/bugs/scripts/soss/tests/test_sossimport.py
> index 675da82..d939dde 100644
> --- a/lib/lp/bugs/scripts/soss/tests/test_sossimport.py
> +++ b/lib/lp/bugs/scripts/soss/tests/test_sossimport.py
> @@ -69,6 +69,10 @@ class TestSOSSImporter(TestCaseWithFactory):
>          pyyaml = self.source_package_name_set.getOrCreateByName("pyyaml")
>          ray = self.source_package_name_set.getOrCreateByName("ray")
>          vllm = self.source_package_name_set.getOrCreateByName("vllm")
> +        extra_attry = {

typo in `extra_attry`, same below

> +            "Custom_attr_1_nvd_score_arrivial_time": "2025-08-03T23:21:39Z",
> +            "Custom_attr_2_assigned_to_someone_time": "2025-08-10T11:22:33Z",
> +        }
>  
>          self.bugtask_reference = [
>              (
> @@ -378,7 +400,10 @@ class TestSOSSImporter(TestCaseWithFactory):
>              ),
>              BugTaskStatus.DEFERRED,
>              "test note",
> -            {"repositories": ["test-repo"]},
> +            {
> +                "repositories": ["test-repo"],
> +                "extra_attrs": self.bugtask_reference[2][3].get("extra_attrs"),

This feels weird - it's hard to tell what `self.bugtask_reference[2][3]` is here without looking at the code, so I'd define `extra_attrs` be outside of here (or get the original value?) and just do `"extra_attrs": extra_attrs`

But also, "extra_attrs remains the same as it is bound to the bug" - what does that mean? If `repositories` and `metadata` are tied to a BugTask, `extra_attrs` shouldn't be tied to a Bug, it should be tied similarly. Else, we should add it as a `Bug` field instead.

Can you clarify what you mean here, or clarify where this attribute should live?

> +            },  # extra_attrs remains the same as it is bound to the bug
>          )
>  
>          soss_importer._create_or_update_bugtasks(bug, self.soss_record)


-- 
https://code.launchpad.net/~ilkeremrekoc/launchpad/+git/launchpad/+merge/493278
Your team Launchpad code reviewers is requested to review the proposed merge of ~ilkeremrekoc/launchpad:add-extra-attrs into launchpad:master.