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

datastore: update postgres package #1092

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft

Conversation

hdonnay
Copy link
Member

@hdonnay hdonnay commented Oct 6, 2023

These changes are the result of trying to go through and apply a uniform style to the datastore/postgres package.

TODO

  • metrics pass
  • coverage pass -- Need to audit the span names
  • preamble/boilerplate pass
  • move ctxlock/v2 to a locksource hierarchy?
  • split the "implement" giant commit (started)
  • undo use of the slices package
  • downgrade the otel packages to 1.18 (for Clair reasons -- it needs Jaeger export support) (It works just fine to have Clair carry the Jaeger requirement and make MVS do the work.)
  • Implement the delta interface

@hdonnay hdonnay force-pushed the hack/db-updates branch 2 times, most recently from 82e3e3c to 26b9e6d Compare October 12, 2023 21:23
@codecov
Copy link

codecov bot commented Oct 12, 2023

Codecov Report

Attention: Patch coverage is 66.67845% with 943 lines in your changes are missing coverage. Please review.

Project coverage is 54.32%. Comparing base (b28ae8c) to head (a2c16b0).

❗ Current head a2c16b0 differs from pull request most recent head 123b1f6. Consider uploading reports for the commit 123b1f6 to get more accurate results

Files Patch % Lines
...atastore/postgres/v2/matcher_v1_updateoperation.go 49.16% 108 Missing and 14 partials ⚠️
datastore/postgres/v2/matcher_v1_enrichment.go 0.00% 110 Missing ⚠️
...atastore/postgres/v2/matcher_v1_vulnerabilities.go 70.85% 87 Missing and 15 partials ⚠️
locksource/pglock/pglock.go 63.52% 81 Missing and 12 partials ⚠️
datastore/postgres/v2/common.go 71.11% 60 Missing and 18 partials ⚠️
datastore/postgres/v2/indexer_v1_manifest.go 57.25% 38 Missing and 15 partials ⚠️
datastore/postgres/v2/indexer_v1_index.go 74.50% 42 Missing and 9 partials ⚠️
datastore/postgres/v2/matcher_v1.go 38.27% 43 Missing and 7 partials ⚠️
...astore/postgres/v2/indexer_v1_affectedmanifests.go 75.50% 38 Missing and 11 partials ⚠️
datastore/postgres/v2/metrics.go 76.96% 34 Missing and 4 partials ⚠️
... and 14 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1092      +/-   ##
==========================================
- Coverage   55.79%   54.32%   -1.48%     
==========================================
  Files         265      249      -16     
  Lines       16552    19713    +3161     
==========================================
+ Hits         9236    10709    +1473     
- Misses       6357     8043    +1686     
- Partials      959      961       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hdonnay hdonnay force-pushed the hack/db-updates branch 4 times, most recently from 0cd0bb1 to a08b31c Compare October 17, 2023 21:39
@hdonnay
Copy link
Member Author

hdonnay commented Oct 17, 2023

@crozzy regarding the big-boy "add new package" commit: any opinion on how to split it up? I was thinking the common code and associated tests in a few commits, then the matcher and indexer implementations and tests in separate commits.

@hdonnay hdonnay force-pushed the hack/db-updates branch 4 times, most recently from 9b929c8 to 11e2f8c Compare October 20, 2023 17:52
@hdonnay hdonnay force-pushed the hack/db-updates branch 2 times, most recently from 95d1cd2 to 9d1f0ae Compare October 31, 2023 20:08
@hdonnay hdonnay marked this pull request as ready for review October 31, 2023 20:28
@hdonnay hdonnay requested a review from a team as a code owner October 31, 2023 20:28
@hdonnay hdonnay requested review from crozzy and removed request for a team October 31, 2023 20:28
@hdonnay
Copy link
Member Author

hdonnay commented Oct 31, 2023

I think is is good to get reviewed. I think the tests need a later pass to work through the Sequence Scan test.

Finally.

This copies the interface out of the `updater` package and then adds a
process-local implementation that's simpler than the one in that
package.

Signed-off-by: Hank Donnay <[email protected]>
This renames the `MatcherStore` to a versioned name, namespaces the
relevant supporting types, and adds aliases to retain API
compatibility.

Signed-off-by: Hank Donnay <[email protected]>
Signed-off-by: Hank Donnay <[email protected]>
Signed-off-by: Hank Donnay <[email protected]>
Signed-off-by: Hank Donnay <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-changelog Label for PRs that need a changelog note.
Development

Successfully merging this pull request may close these issues.

1 participant