Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make bulk search fast #1017

Merged
merged 5 commits into from
Nov 28, 2022
Merged

Make bulk search fast #1017

merged 5 commits into from
Nov 28, 2022

Conversation

TG1999
Copy link
Contributor

@TG1999 TG1999 commented Nov 25, 2022

Signed-off-by: Tushar Goel [email protected]

@TG1999
Make bulk search fast
44248b5 
Signed-off-by: Tushar Goel <[email protected]>
@TG1999 TG1999 mentioned this pull request Nov 25, 2022
Closed
pombredanne
pombredanne requested changes Nov 25, 2022
View reviewed changes
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

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

Thanks!
See feedback inline.

vulnerabilities/migrations/0034_package_package.py Outdated Show resolved Hide resolved
vulnerabilities/migrations/0034_package_package.py Outdated Show resolved Hide resolved
vulnerabilities/models.py Outdated Show resolved Hide resolved
vulnerabilities/models.py Outdated Show resolved Hide resolved
vulnerabilities/migrations/0035_auto_20221125_1257.py Outdated Show resolved Hide resolved
vulnerabilities/migrations/0035_auto_20221125_1257.py Outdated Show resolved Hide resolved
@TG1999 TG1999 marked this pull request as draft November 25, 2022 15:50
@TG1999
Copy link
Contributor Author

TG1999 commented Nov 25, 2022

@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 <[email protected]>
tdruez
tdruez reviewed Nov 28, 2022
View reviewed changes
vulnerabilities/api.py Outdated Show resolved Hide resolved
@tdruez
Copy link
Contributor

tdruez commented Nov 28, 2022

@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 TG1999 marked this pull request as ready for review November 28, 2022 13:54
@TG1999
Add CHANGELOG
44939b0 
Signed-off-by: Tushar Goel <[email protected]>
pombredanne
pombredanne reviewed Nov 28, 2022
View reviewed changes
vulnerabilities/models.py
db_index=True,
)

plain_package_url = models.CharField(
Copy link
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
Copy link
Member

@pombredanne pombredanne left a comment

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

@pombredanne pombredanne pombredanne approved these changes

@tdruez tdruez tdruez 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