Code review comment for ~litios/ubuntu-cve-tracker:tags-patches-publishing-cve

Revision history for this message
Camila Camargo de Matos (ccdm94) wrote (last edit ):

I am by no means a cve_lib expert, but after some explanation from @litios, and some testing using the sample code provided, this LGTM.

The tags change might seem like the correct way to go about this, since with the change, if there are no tags, the field will be empty, as expected if there are no tags (when I say "expected", I mean, in theory). I am just not sure if this would break anything that uses the tags information and expects to receive a non-empty dictionary, even if the non-empty dictionary only contains keys with empty lists, so that might be worth checking out.

After initial analysis, it seems like from our side nothing would break, as we have the following:
  - for the 'tags' key being accessed in `html_export.py`: we have the `if pkg in data['tags']:` instruction, so there is a check here before accessing the tag information. The loop just won't execute in this case (at all), and what is executed inside the loop does not look like something that will affect the rest of the code negatively.
  - for the 'tags' key being accessed in `cve_lib`: the access seems to occur on top of data recovered directly from a CVE file through `load_cves`, so I guess the webpage data being different wouldn't cause any issues in the execution of functions from `cve_lib`, since the webpage that is not what is being used as a reference.

« Back to merge proposal