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 flush api #1895

Closed
wants to merge 29 commits into from
Closed

Conversation

aknishid
Copy link
Contributor

@aknishid aknishid commented Dec 15, 2022

Description:

Flush API to delete all vectors has been added.

Related Issue:

Versions:

  • Go Version: 1.19.4
  • Docker Version: 20.10.8
  • Kubernetes Version: 1.22.0
  • NGT Version: 1.14.8

Checklist:

Special notes for your reviewer:

@CLAassistant
Copy link

CLAassistant commented Dec 15, 2022

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.
3 out of 4 committers have signed the CLA.

✅ takuyaymd
✅ aknishid
✅ kpango
❌ takuyyam


takuyyam seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@codecov
Copy link

codecov bot commented Dec 16, 2022

Codecov Report

Patch coverage: 1.67% and project coverage change: -0.43 ⚠️

Comparison is base (6f5fb18) 31.09% compared to head (355ae00) 30.67%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1895      +/-   ##
==========================================
- Coverage   31.09%   30.67%   -0.43%     
==========================================
  Files         339      340       +1     
  Lines       32831    33295     +464     
==========================================
+ Hits        10210    10212       +2     
- Misses      22169    22615     +446     
- Partials      452      468      +16     
Impacted Files Coverage Δ
internal/client/v1/client/vald/vald.go 0.00% <0.00%> (ø)
internal/core/algorithm/ngt/ngt.go 61.57% <0.00%> (-1.17%) ⬇️
internal/errors/ngt.go 70.00% <ø> (ø)
internal/file/file.go 11.56% <0.00%> (-1.14%) ⬇️
pkg/agent/core/ngt/handler/grpc/flush.go 0.00% <0.00%> (ø)
pkg/agent/core/ngt/handler/grpc/insert.go 60.61% <0.00%> (-7.18%) ⬇️
pkg/agent/core/ngt/handler/grpc/linear_search.go 0.00% <0.00%> (ø)
pkg/agent/core/ngt/handler/grpc/remove.go 22.66% <0.00%> (-3.21%) ⬇️
pkg/agent/core/ngt/handler/grpc/search.go 22.00% <0.00%> (-1.11%) ⬇️
pkg/agent/core/ngt/handler/grpc/update.go 27.61% <0.00%> (-2.63%) ⬇️
... and 3 more

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

return err
}
eg.Go(safety.RecoverFunc(func() (err error) {
_, err = os.Delete(childPath)
Copy link

Choose a reason for hiding this comment

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

59% of developers fix this issue

typecheck: Delete not declared by package os


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

return err
}
eg.Go(safety.RecoverFunc(func() (err error) {
_, err = os.Remove(childPath)
Copy link

Choose a reason for hiding this comment

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

59% of developers fix this issue

typecheck: cannot assign 1 values to 2 variables


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@@ -21,6 +21,9 @@ var (
// ErrCreateIndexingIsInProgress represents an error that the indexing is in progress but search request received
Copy link

Choose a reason for hiding this comment

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

godot: Comment should end in a period


ℹ️ Learn about @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

internal/errors/ngt.go Outdated Show resolved Hide resolved
@@ -18,6 +18,91 @@
package errors

var (
// ErrCreateIndexingIsInProgress represents an error that the indexing is in progress but search request received
ErrCreateIndexingIsInProgress = New("create indexing is in progress")
Copy link

Choose a reason for hiding this comment

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

27% of developers fix this issue

typecheck: ErrCreateIndexingIsInProgress redeclared in this block

❗❗ 10 similar findings have been found in this PR

🔎 Expand here to view all instances of this finding
File Path Line Number
internal/errors/ngt.go 28
internal/errors/ngt.go 33
internal/errors/ngt.go 36
internal/errors/ngt.go 39
internal/errors/ngt.go 42
internal/errors/ngt.go 50
internal/errors/ngt.go 55
internal/errors/ngt.go 60
internal/errors/ngt.go 65
internal/errors/ngt.go 68

Visit the Lift Web Console to find more details in your report.


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Help us improve LIFT! (Sonatype LiftBot external survey)

Was this a good recommendation for you? Answering this survey will not impact your Lift settings.

[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@aknishid aknishid force-pushed the feature/apis/add-flush-api branch from 961b327 to 0393f72 Compare April 7, 2023 06:36
ErrUnsupportedDistanceType = New("unsupported DistanceType")

// ErrFailedToSetDistanceType represents a function to generate an error that the set of distance type failed.
ErrFailedToSetDistanceType = func(err error, distance string) error {
Copy link

Choose a reason for hiding this comment

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

21% of developers fix this issue

typecheck: ErrFailedToSetDistanceType redeclared in this block

❗❗ 9 similar findings have been found in this PR

🔎 Expand here to view all instances of this finding
File Path Line Number
internal/errors/ngt.go 76
internal/errors/ngt.go 81
internal/errors/ngt.go 86
internal/errors/ngt.go 91
internal/errors/ngt.go 96
internal/errors/ngt.go 101
internal/errors/ngt.go 104
internal/file/file.go 22
internal/file/file.go 505

Visit the Lift Web Console to find more details in your report.


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.


Help us improve LIFT! (Sonatype LiftBot external survey)

Was this a good recommendation for you? Answering this survey will not impact your Lift settings.

[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

@aknishid aknishid force-pushed the feature/apis/add-flush-api branch from d6f247a to a8c8cb6 Compare April 25, 2023 02:15
@aknishid aknishid force-pushed the feature/apis/add-flush-api branch 4 times, most recently from 23a213b to 7a234aa Compare May 23, 2023 01:37
@aknishid aknishid force-pushed the feature/apis/add-flush-api branch 2 times, most recently from 4013166 to 8ad592a Compare May 23, 2023 10:20
datelier
datelier previously approved these changes Jul 6, 2023
kpango
kpango previously approved these changes Jul 20, 2023
Copy link
Collaborator

@kpango kpango left a comment

Choose a reason for hiding this comment

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

LGTM

@kpango kpango dismissed stale reviews from datelier, vankichi, and themself via 355ae00 July 20, 2023 06:18
Copy link
Collaborator

@kpango kpango left a comment

Choose a reason for hiding this comment

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

LGTM

@aknishid
Copy link
Contributor Author

I've opened a new PR (#2273), so I will be closing this one.

@aknishid aknishid closed this Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants