Skip to content

Make bulk search fast#1017

Merged
pombredanne merged 5 commits into
aboutcode-org:mainfrom
TG1999:fast_bulk_search
Nov 28, 2022
Merged

Make bulk search fast#1017
pombredanne merged 5 commits into
aboutcode-org:mainfrom
TG1999:fast_bulk_search

Conversation

@TG1999

@TG1999 TG1999 commented Nov 25, 2022

Copy link
Copy Markdown
Contributor

Signed-off-by: Tushar Goel tushar.goel.dav@gmail.com

@TG1999
Make bulk search fast
44248b5 
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
@TG1999 TG1999 mentioned this pull request Nov 25, 2022
Closed
pombredanne
pombredanne requested changes Nov 25, 2022
View reviewed changes

@pombredanne pombredanne left a comment

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.

Thanks!
See feedback inline.

Comment thread vulnerabilities/migrations/0034_package_package.py Outdated
Comment thread vulnerabilities/migrations/0034_package_package.py Outdated
Comment thread vulnerabilities/models.py Outdated
Comment thread vulnerabilities/models.py Outdated
Comment thread vulnerabilities/migrations/0035_auto_20221125_1257.py Outdated
Comment thread vulnerabilities/migrations/0035_auto_20221125_1257.py Outdated
@TG1999
TG1999 marked this pull request as draft November 25, 2022 15:50
@TG1999

TG1999 commented Nov 25, 2022

Copy link
Copy Markdown
Contributor Author

@pombredanne the tests where we ignore qualifiers and subpath are failing because we are matching purls directly without removing qualifiers and subpath, what I suggest as a solution is to store purl_without_qualifiers_subpath as a different field and remove qualifiers and subpath from incoming purls and match the list like we are doing now.

@TG1999
Address review comments
17317ca 
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
tdruez
tdruez reviewed Nov 28, 2022
View reviewed changes
Comment thread vulnerabilities/api.py Outdated
@tdruez

tdruez commented Nov 28, 2022

Copy link
Copy Markdown
Contributor

@pombredanne the tests where we ignore qualifiers and subpath are failing because we are matching purls directly without removing qualifiers and subpath, what I suggest as a solution is to store purl_without_qualifiers_subpath as a different field and remove qualifiers and subpath from incoming purls and match the list like we are doing now.

Yes, you should store both full purl and plain purl and allow to run the matching on the plain purl.

@TG1999
Add plain_purl option and add tests
c22de9c 
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
@TG1999
TG1999 marked this pull request as ready for review November 28, 2022 13:54
@TG1999
Add CHANGELOG
44939b0 
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
pombredanne
pombredanne reviewed Nov 28, 2022
View reviewed changes
Comment thread vulnerabilities/models.py
db_index=True,
)

plain_package_url = models.CharField(

@pombredanne pombredanne Nov 28, 2022

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.

I am not in love with the plain_package_url name but this makes sense for now. We will need to find a better way.
I will merge for now!

pombredanne
pombredanne approved these changes Nov 28, 2022
View reviewed changes

@pombredanne pombredanne left a comment

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.

Thanks! This will need to be refined name wise but this is a good fix for now.
Merging

@pombredanne
pombredanne merged commit 768bdd0 into aboutcode-org:main Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tdruez tdruez tdruez approved these changes
@pombredanne pombredanne pombredanne approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@TG1999 @tdruez @pombredanne