Skip to content

#6 Debian & #7 Ubuntu Scrapers - #14

Merged
tdruez merged 26 commits into
developfrom
scraper
Jul 14, 2017
Merged

#6 Debian & #7 Ubuntu Scrapers#14
tdruez merged 26 commits into
developfrom
scraper

Conversation

@kartiksibal

@kartiksibal kartiksibal commented Jul 7, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

What it does:

  1. Gets the job done.

Things remaining:

  1. Add test cases.
  2. Remove boilerplate.

@kartiksibal
Style changes in Debian #6
05ec4d6 
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
@kartiksibal
Style changes iUbuntu #7
e7e8997 
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
@kartiksibal kartiksibal changed the title #6 Debian & Ubuntu Scrapers #6 Debian & #7 Ubuntu Scrapers Jul 7, 2017
kartiksibal and others added 9 commits July 7, 2017 09:39
@kartiksibal kartiksibal self-assigned this Jul 11, 2017
kartiksibal
kartiksibal commented Jul 11, 2017
View reviewed changes
Comment thread test_scrapers.py Outdated
# Fix Me: The test data doesn't accurately depict the
# actual debian website on which, the code under testing
# is written
from scraper import debian

@kartiksibal kartiksibal Jul 11, 2017 •
edited
Loading

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.

@tdruez @pombredanne The main debian website (https://security-tracker.debian.org/tracker/), has several child datasets eg: https://security-tracker.debian.org/tracker/status/release/unstable.

The scraping code, firsts visits the main website, extracts the child links, and then visits the child links to extract data. I couldn't form a test data, that could depict this behaviour.

What do you suggest?

@tdruez tdruez Jul 11, 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.

firsts visits the main website, extracts the child links, and then visits the child links to extract data.

Then I suggest those 3 logics should be in 3 separate functions, this would make thing much easier to test as well.

tdruez
tdruez reviewed Jul 11, 2017
View reviewed changes
Comment thread scraper/debian.py Outdated


def debian_data():
cve_id = []

@tdruez tdruez Jul 11, 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.

Those variables are out of their scope.

Comment thread scraper/debian.py Outdated

for child_links in range(6):
# Extract package info from all the child datasets
child_url = urlopen("https://security-tracker.debian.org"

@tdruez tdruez Jul 11, 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.

This part should be refactored into its own function.

tdruez
tdruez reviewed Jul 11, 2017
View reviewed changes
Comment thread test_scrapers.py Outdated
# Fix Me: The test data doesn't accurately depict the
# actual debian website on which, the code under testing
# is written
from scraper import debian

@tdruez tdruez Jul 11, 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.

Why is the import in the function instead of on top of the module?

tdruez
tdruez reviewed Jul 11, 2017
View reviewed changes
Comment thread test_scrapers.py Outdated
# Visit https://github.com/nexB/vulnerablecode/ for support and download.

import bs4 as bs
from mock import Mock

@tdruez tdruez Jul 11, 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.

Remove un-used imports.

tdruez and others added 8 commits July 11, 2017 10:55
@tdruez
Add lxml in the requirements.txt #6
980265c 
Signed-off-by: Thomas Druez <tdruez@nexb.com>
@kartiksibal
Debian test case and style changes #6
e7af960 
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
@kartiksibal
yMerge branch 'scraper' of https://github.com/nexB/vulnerablecode int…
84b4a2c 
…o scraper
@kartiksibal
Minor changes #6
9b34f7a 
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
@kartiksibal
Minor style changes #5
fee110a 
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
@kartiksibal
Merge branch 'models_db' of https://github.com/nexB/vulnerablecode in…
f366133 
…to models_db
@kartiksibal
Minor changes #5
0b70285 
Signed-off-by: Kartik Sibal <kartiksibal@gmail.com>
@tdruez
Merge branch 'develop' into scraper
b5e5ba3
tdruez added 5 commits July 13, 2017 11:57
@tdruez
Move tests files in a tests/ directory #6
6569bb2 
- Add tests runner in the travis config
- Cleanup

Signed-off-by: Thomas Druez <tdruez@nexb.com>
@tdruez
Merge branch 'models_db' into scraper
70711c0
@tdruez
Refactor and refine the code and structure for scrapers #6
4abd00b 
Signed-off-by: Thomas Druez <tdruez@nexb.com>
@tdruez
Merge branch 'models_db' of github.com:nexB/vulnerablecode into model…
11c7954 
…s_db
@tdruez
Simplify status condition in extract_cves_from_tracker #6
f0e46ce 
Signed-off-by: Thomas Druez <tdruez@nexb.com>
tdruez
tdruez requested changes Jul 14, 2017
View reviewed changes
Comment thread scraper/debian.py

# if package name is empty, use the previous package name
if href == '/tracker/source-package/':
package_name.append(pkg)

@tdruez tdruez Jul 14, 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.

Warning: The pkg may be referenced before assignment.
IE: if the previous if condition did not check, pkg will not be available.

@tdruez

tdruez commented Jul 14, 2017

Copy link
Copy Markdown
Contributor

@kartiksibal the current data structure for the returned CVEs is not easily usable, instead of the current 3 lists of strings, we should return a single list of CVEs dictionaries.

Instead of:

(
    ['CVE-2016-5416', 'CVE-2012-6655'],  
    ['389-ds-base', 'accountsservice'],  
    ['not yet assigned', 'not yet assigned'],
)

Return:

[
    {
        'cve_id': 'CVE-2016-5416', 
        'package_name': '389-ds-base', 
        'vulnerability_status': 'not yet assigned',
    },
    {
        'cve_id': 'CVE-2012-6655', 
        'package_name': 'accountsservice', 
        'vulnerability_status': 'not yet assigned',
    },
]

It will be much easier to use that data structure to then create model instances (save the data in the DB).

@tdruez
tdruez merged commit f0e46ce into develop Jul 14, 2017
@tdruez
tdruez deleted the scraper branch July 14, 2017 00:34
@tdruez tdruez mentioned this pull request Jul 14, 2017
Closed
pombredanne pushed a commit that referenced this pull request Apr 2, 2025
@JonoYang
Merge pull request #14 from nexB/develop
5593c2f 
Merge changes from develop to main
pombredanne added a commit that referenced this pull request Apr 2, 2025
@pombredanne
Merge pull request #14 from nexB/refinements
dd188e4 
Add support for gems and improve RPM support
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tdruez tdruez tdruez requested changes
@pombredanne pombredanne Awaiting requested review from pombredanne

Assignees

@kartiksibal kartiksibal

Labels

enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kartiksibal @tdruez