Skip to content

Refactor scraper logic and datastructure #16 - #17

Merged
tdruez merged 19 commits into
developfrom
scraper_datastructure
Jul 26, 2017
Merged

Refactor scraper logic and datastructure #16#17
tdruez merged 19 commits into
developfrom
scraper_datastructure

Conversation

@tdruez

@tdruez tdruez commented Jul 20, 2017

Copy link
Copy Markdown
Contributor
  • All data is scraped in 1 loop
  • Remove the need for regex
  • Return a dict datastructure
  • Enhanced the tests using input as collected on the source

Signed-off-by: Thomas Druez tdruez@nexb.com

- All data is scraped in 1 loop
- Remove the need for regex
- Return a dict datastructure
- Enhanced the tests using input as collected on the source

Signed-off-by: Thomas Druez <tdruez@nexb.com>
@kartiksibal

Copy link
Copy Markdown
Contributor

@tdruez Looks great! 👍
We'll get to Debian in a little while? @pombredanne

Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
Comment thread scraper/debian.py Outdated
fields_names = ['status', 'urgency', 'fixed_version']

return cve_id, package_name, vulnerability_status
return [{name: version_detail.get(name) for name in fields_names}

@kartiksibal kartiksibal Jul 23, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pombredanne @tdruez This will get the three mentioned fields, but obviously not Package Name & Vulnerability. I can't seem to return data in {Dicitonary[list{dicitonary}]} which is {package_name [cve-ids{cve_id_data}]} format.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not ever use complicated and complex comprehensions with more than a single level.
Use regular loops instead.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kartiksibal FWIW, @tdruez recommendation to use a dict comprehension was for when you have a more simple, single-level iteration... not multiple level nested loops

Comment thread scraper/debian.py Outdated
def extract_cves_from_tracker(html):
def debian_extract_data():
"""
Return all CVEs extracted from the given `html` input.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring is no longer up to date

Comment thread scraper/debian.py Outdated
# if package name is empty, use the previous package name
if href == '/tracker/source-package/':
package_name.append(pkg)
test_input = urlopen("https://security-tracker.debian.org/tracker/data/json").read()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to load this with the json module...
e.g. once you have read the content, call debian_vulnerabilities = json.loads(test_input)

Comment thread scraper/debian.py Outdated
fields_names = ['status', 'urgency', 'fixed_version']

return cve_id, package_name, vulnerability_status
return [{name: version_detail.get(name) for name in fields_names}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not ever use complicated and complex comprehensions with more than a single level.
Use regular loops instead.

kartiksibal and others added 9 commits July 24, 2017 04:55
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
Signed-off-by: kartik sibal <kartiksibal@gmail.com>
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
Signed-off-by: Thomas Druez <tdruez@nexb.com>
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
Signed-off-by: Kartik Sibal
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
Comment thread scraper/debian.py Outdated
for distro, version_detail in details.get('releases', {}).items():
if distro == 'jessie':
final_data.append({
"package_name": package_name,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use single quotes except for doc strings and use all lower case keys.

Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
Comment thread tests/test_scrapers.py Outdated
'vulnerability_id': 'TEMP-0807341-84E914'
},

{

@tdruez tdruez Jul 25, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Collecting this have no value.
If the 'jessie' data is not available we should not create an entry in the output.

I'm talking about the sysvinit entry.

kartiksibal and others added 7 commits July 26, 2017 01:01
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
 * organize and sort imports
 * simplify reading test data from JSON
 * format expectation

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
 * use global URL as a default for a json_data() url arg
 * use base_release as an arg for extract_data() with default to jessie
 * do not use fragile chained get in extract_data() and check for
   possible empty values ahead of use
 * use more descriptive variable names
 * improve docstrings
 * organize imports and improve code formatting

Signed-off-by: Philippe Ombredanne <pombredanne@nexb.com>
@tdruez
tdruez merged commit 3d78b73 into develop Jul 26, 2017
@tdruez
tdruez deleted the scraper_datastructure branch July 26, 2017 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants