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

Add EPSS Ingestor command to ingest-worker #1070

Merged
merged 9 commits into from Jan 18, 2023
Merged

Conversation

freeqaz
Copy link
Member

@freeqaz freeqaz commented Jan 2, 2023

Detailed Summary:

  • This commit adds a new epss command to the ingest-worker.
  • The epss command includes an "ingest" subcommand which allows the user to update EPSS scores.
  • The epss command is implemented using the clifx library and includes a NewCommand function to create the cli.Command struct.
  • The epss command has a dependency on an EPSSIngester interface which is provided through the fx dependency injection library.
  • The epss command logs informative messages when starting and completing the ingestion process.

(Thank ChatGPT for the git commit message lol)

The SQL I wrote is the fastest that I could figure out. The first implementation took >10 minutes and this one takes about 30 seconds now.

It turns out that query for the IDs ahead of time and then running an "UPDATE" for that is the fastest way. (worth noting)

Detailed Summary:

    - This commit adds a new epss command to the ingest-worker.
    - The epss command includes an "ingest" subcommand which allows the user to update EPSS scores.
    - The epss command is implemented using the clifx library and includes a NewCommand function to create the cli.Command struct.
    - The epss command has a dependency on an EPSSIngester interface which is provided through the fx dependency injection library.
    - The epss command logs informative messages when starting and completing the ingestion process.

(Thank ChatGPT for the git commit message lol)

The SQL I wrote is the fastest that I could figure out. The first implementation took >10 minutes and this one takes about 30 seconds now.

It turns out that query for the IDs ahead of time and then running an "UPDATE" for that is the fastest way. (worth noting)

// Get all CVEs and GHSAs from the database
cvePairs, err := tx.QueryContext(ctx, `
SELECT v1.source_id, v2.source_id FROM vulnerability.equivalent e
Copy link
Contributor

Choose a reason for hiding this comment

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

im glad this works for now, but there must be a real library we can use for this in the future. passing the raw sql strings is starting to look clunky, although god knows ive done it a lot myself

Copy link
Contributor

Choose a reason for hiding this comment

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

In node there are quite a few query builder libraries. There are also libraries which will read in .sql files and template stuff into them. That has the advantage of code separation, and it will allow the IDE to work some autocomplete magic on the SQL. We might consider that in the future

Copy link
Member Author

Choose a reason for hiding this comment

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

Screenshot_2023-01-03_15-54-53

The SQL autocomplete works great for me FWIW

Copy link
Contributor

@ajvpot ajvpot left a comment

Choose a reason for hiding this comment

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

please run go fmt on your changes, they don't match the language style specifically use tabs for indentation and there is no limit on line length.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ breadchris
✅ freeqaz
❌ github-actions[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

@breadchris breadchris merged commit 3a064c1 into master Jan 18, 2023
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

5 participants