Skip to content

Add API/ UI support for Patch/PackageCommitPatch#2179

Merged
TG1999 merged 7 commits into
aboutcode-org:mainfrom
ziadhany:api-commit-patch
Jun 15, 2026
Merged

Add API/ UI support for Patch/PackageCommitPatch#2179
TG1999 merged 7 commits into
aboutcode-org:mainfrom
ziadhany:api-commit-patch

Conversation

@ziadhany

@ziadhany ziadhany commented Feb 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

@ziadhany ziadhany force-pushed the api-commit-patch branch 2 times, most recently from 65506e3 to d1b139f Compare March 3, 2026 01:11
@ziadhany ziadhany requested review from TG1999 and keshav-space March 3, 2026 01:23
@ziadhany ziadhany mentioned this pull request Apr 14, 2026
Closed
4 tasks
@ziadhany ziadhany mentioned this pull request Apr 30, 2026
Merged
@ziadhany ziadhany force-pushed the api-commit-patch branch from fef275d to 227c0df Compare May 5, 2026 11:26
@TG1999 TG1999 marked this pull request as draft May 14, 2026 10:33
@ziadhany ziadhany force-pushed the api-commit-patch branch 2 times, most recently from 227c0df to 20b2637 Compare May 14, 2026 13:16
TG1999
TG1999 reviewed May 14, 2026
View reviewed changes
Comment thread vulnerablecode/urls.py Outdated
@ziadhany ziadhany force-pushed the api-commit-patch branch 3 times, most recently from e5aa2a0 to a0db697 Compare June 1, 2026 10:50
@ziadhany ziadhany marked this pull request as ready for review June 1, 2026 10:51
@TG1999

TG1999 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

@ziadhany thanks for this, please rebase and change your approach according to the latest code. Thanks!

ziadhany added 2 commits June 11, 2026 11:28
@ziadhany
Add API support for PackageCommitPatch
94a5f10 
Add a test

Signed-off-by: ziad hany <ziadhany2016@gmail.com>
@ziadhany
Resolve merge conflict
f0cbcbc 
Signed-off-by: ziad hany <ziadhany2016@gmail.com>
@ziadhany
Fix a typo in AdvisoryPackageCommitPatchDetails
f5ff6d8 
Signed-off-by: ziad hany <ziadhany2016@gmail.com>
@ziadhany ziadhany requested a review from TG1999 June 12, 2026 08:31
TG1999
TG1999 reviewed Jun 12, 2026
View reviewed changes
Comment thread vulnerabilities/api_v3.py Outdated
@ziadhany
Update the Patch api to not use JSONBAgg
bf962b0 
Signed-off-by: ziad hany <ziadhany2016@gmail.com>
TG1999
TG1999 reviewed Jun 15, 2026
View reviewed changes
Comment thread vulnerabilities/tests/test_api_v3.py Outdated
TG1999
TG1999 reviewed Jun 15, 2026
View reviewed changes
Comment thread vulnerabilities/api_v3.py Outdated
@ziadhany
Update api patches to use all_advisory_ids
c486942 
Add a test for member advisory
Simplify the code to use one query

Signed-off-by: ziad hany <ziadhany2016@gmail.com>
@ziadhany

ziadhany commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator Author

@TG1999 I just updated the code as we discussed.

For the smaller query:

With reachability:
    CPU: 104.13 ms (145.20 ms)
    SQL: 13 queries in 19.43 ms

Without reachability:
    CPU: 66.83 ms (94.41 ms)
    SQL: 12 queries in 19.27 ms

For the larger query:


With reachability:
    SQL: 213 queries in 1288.40 ms
    CPU: 1743.61 ms (2887.35 ms)

Without reachability:
    SQL: 212 queries in 1333.57 ms
    CPU: 1724.57 ms (2884.75 ms)

@ziadhany ziadhany requested a review from TG1999 June 15, 2026 12:02
TG1999
TG1999 reviewed Jun 15, 2026
View reviewed changes
Comment thread vulnerabilities/api_v3.py Outdated
fixed_commit_hash=F("impacted_package__fixed_by_package_commit_patches__commit_hash"),
fixed_vcs_url=F("impacted_package__fixed_by_package_commit_patches__vcs_url"),
)
.distinct()

@TG1999 TG1999 Jun 15, 2026

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.

Try removing distinct and remove seen data in the for loop and compare the performance once

@TG1999 TG1999 Jun 15, 2026

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.

Let's have 2 separate queries for affected anf fixed that's fine

@TG1999 TG1999 Jun 15, 2026

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. 

introduced_rows = (
    ImpactedPackageAffecting.objects
    ...
    .filter(
        impacted_package__introduced_by_package_commit_patches__isnull=False
    )
    .values(...)
)

fixed_rows = (
    ImpactedPackageAffecting.objects
    ...
    .filter(
        impacted_package__fixed_by_package_commit_patches__isnull=False
    )
    .values(...)
)

@ziadhany ziadhany Jun 15, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

after remove the distinct and remove seen, the query slightly faster on large query, but it results in duplicate patches.

@TG1999 TG1999 Jun 15, 2026

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.

What I meant is use seen approach in loop without distinct,

Remove dupes from loop and see how it goes

@ziadhany ziadhany Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

There is no obvious difference in performance. I just dropped distinct and kept the seen loop.

TG1999
TG1999 reviewed Jun 15, 2026
View reviewed changes
Comment thread vulnerabilities/api_v3.py Outdated
TG1999
TG1999 reviewed Jun 15, 2026
View reviewed changes
Comment thread vulnerabilities/api_v3.py Outdated
TG1999
TG1999 reviewed Jun 15, 2026
View reviewed changes
Comment thread vulnerabilities/api_v3.py Outdated
@ziadhany
Update the patches api to use two queries
3eab3e9 
Simplify the code structure
Prepare the patch data outside the loop

Signed-off-by: ziad hany <ziadhany2016@gmail.com>
@ziadhany

ziadhany commented Jun 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

@TG1999, your idea was brilliant 🚀🚀 . The performance of the larger query is now:

With reachability:
CPU: 810.10ms (1404.72ms)
SQL: 214 queries in 657.66ms

Without reachability:
CPU: 776.16 (1411.5 ms)
SQL: 212 queries in 688ms

This means:

1,404 / 2,887 = 48.6% improvement

also:

For reachability, true the difference in the SQL query count is just 2 queries.

Please let me know if you have any other suggestions.

TG1999
TG1999 approved these changes Jun 15, 2026
View reviewed changes

@TG1999 TG1999 left a comment

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.

LGTM!

@TG1999 TG1999 merged commit c6aff9c into aboutcode-org:main Jun 15, 2026
4 checks passed
@ziadhany ziadhany deleted the api-commit-patch branch June 15, 2026 15:17
@ziadhany ziadhany mentioned this pull request Jun 15, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TG1999 TG1999 TG1999 approved these changes

@keshav-space keshav-space Awaiting requested review from keshav-space

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ziadhany @TG1999