Skip to content

feat: add support for an 'upstream' field in the models#3172

Merged
jess-lowe merged 20 commits intogoogle:masterfrom
jess-lowe:up-models
Feb 27, 2025
Merged

feat: add support for an 'upstream' field in the models#3172
jess-lowe merged 20 commits intogoogle:masterfrom
jess-lowe:up-models

Conversation

@jess-lowe
Copy link
Copy Markdown
Contributor

This PR aims to prepare the models.py file to support the 'upstream' field to be introduced by ossf/osv-schema#312. Part of addressing #3052

This will calculate all of the transitive upstream dependencies of a given CVE. Hierarchy will be calculated and determined by the frontend in a later PR.

@jess-lowe jess-lowe requested review from a team and hogo6002 and removed request for a team February 21, 2025 00:58
Copy link
Copy Markdown
Contributor

@another-rex another-rex left a comment

Choose a reason for hiding this comment

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

Some minor code comments here, will discuss the rest in the design doc.

osv/models.py Outdated
@@ -762,7 +780,9 @@ def to_vulnerability(self, include_source=False, include_alias=True):
def to_vulnerability_async(self, include_source=False):
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.

Do we want to add include_upstream and include_alias to the this function

Copy link
Copy Markdown
Contributor

@hogo6002 hogo6002 left a comment

Choose a reason for hiding this comment

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

LGTM!!!

osv/models.py Outdated
# Related IDs.
related: list[str] = ndb.StringProperty(repeated=True)
# Upstream IDs.
upstream: list[str] = ndb.StringProperty(repeated=True)
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.

Please rename this to upstream_raw as described in the doc.

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.

Can you also expand the docstring above this to explain that this is upstream ids directly from the import and does not include any transitive upstreams.

osv/models.py Outdated


class UpstreamGroup(ndb.Model):
"""Upstream group for storing transitive upstreams of a Bug"""
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.

Can you expand the description here to explain why this is in a separate table rather than directly on the bug entry itself?

I.e. to prevent additional race conditions, by having it in a separate table only worker will modify Bug's directly.

Copy link
Copy Markdown
Contributor

@another-rex another-rex left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jess-lowe jess-lowe merged commit 1fe42bf into google:master Feb 27, 2025
11 checks passed
hogo6002 pushed a commit to hogo6002/osv.dev that referenced this pull request Feb 27, 2025
This PR aims to prepare the models.py file to support the 'upstream'
field to be introduced by ossf/osv-schema#312.
Part of addressing google#3052

This will calculate all of the transitive upstream dependencies of a
given CVE. Hierarchy will be calculated and determined by the frontend
in a later PR.
@jess-lowe jess-lowe deleted the up-models branch September 26, 2025 03:00
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