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.

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

refactor: table init + add bonus purl2cpe init #4241

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

terriko
Copy link
Contributor

@terriko terriko commented Jul 3, 2024

purl2cpe was somtimes not being initalized correctly before we added data if the cache was old. Fixing this spawned a refactor to the database init code as well. There's also an opportunity to refactor the schema checks in a similar way later.

This should fix the periodic error we get that looks like this:

│ ❱  504 │   │   purl2cpe_cursor.execute("SELECT purl, cpe FROM purl2cpe")     │
│    505 │   │                                                                 │
│    506 │   │   insert_query = """                                            │
│    507 │   │   │   INSERT INTO purl2cpe (purl, cpe)                          │
╰──────────────────────────────────────────────────────────────────────────────╯
OperationalError: no such table: purl2cpe

purl2cpe was somtimes not being initalized correctly before we added
data if the cache was old.  Fixing this spawned a refactor to the
database init code as well.  There's also an opportunity to refactor the
schema checks in a similar way later.

Signed-off-by: Terri Oda <[email protected]>
Comment on lines +441 to +445
# we are occasionally seeing an error where the cache doesn't have
# purl2cpe and thus we get an error, so attempt to initalize here
cve_cursor.execute(self.TABLE_SCHEMAS["purl2cpe"])
cve_cursor.execute(self.INDEXES["purl"])

Copy link
Contributor

Choose a reason for hiding this comment

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

I proposed a change in commit 622efe8. With this change, we should no longer face this issue. So, it's not necessary, but I don't know we can have it as a added security?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's try your change out first and see how it goes! As in, I'll plan to merge your script before this PR gets completed.

I think I still want the rest of the refactoring in here, though, so I'll continue to work to get that behaving rather than closing this PR. I'll decide later if we need to take out these two lines or (since I don't think they'll add much overhead) if they should just say in as a backup safety check.

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.

None yet

2 participants