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.

Sign up for GitHub

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

Jump to bottom

Add RemoveByTimestamp RPC #2158

Merged
merged 28 commits into from
Aug 31, 2023
Merged

Add RemoveByTimestamp RPC #2158

merged 28 commits into from
Aug 31, 2023

Conversation

hlts2
Copy link
Contributor

@hlts2 hlts2 commented Aug 24, 2023 •
edited
Loading

Description:

I have added a new RPC (RemoveByTimestamp) to remove vectors based on timestamp.

The following conditions can be specified multiple times.

  • Eq: Equal
  • Ne: Not Equal
  • Ge: greater than or equal
  • Gt: greater than
  • Le: less than or equal;
  • Lt: less than

Related Issue:

Versions:

  • Go Version: 1.20.6
  • Docker Version: 20.10.8
  • Kubernetes Version: v1.27.3
  • NGT Version: 2.0.16

Checklist:

Special notes for your reviewer:

@vdaas-ci
Copy link
Collaborator

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 💌 /changelog - replace the PR body by changelog details
  • 🍱 /format - format codes and add licenses
  • ✅ /gen-test - generate test codes
  • 🏷️ /label - add labels
  • ⏪ /rebase - rebase main
  • 🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

@codecov
Copy link

codecov bot commented Aug 24, 2023 •
edited
Loading

Codecov Report

Patch coverage is 24.75% of modified lines.

Files Changed Coverage
internal/client/v1/client/vald/vald.go 0.00%
pkg/gateway/filter/handler/grpc/handler.go 0.00%
pkg/gateway/lb/handler/grpc/handler.go 0.00%
pkg/agent/core/ngt/handler/grpc/remove.go 71.29%

📢 Thoughts on this report? Let us know!.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Aug 24, 2023 •
edited
Loading

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: d32d2a9
Status: ✅  Deploy successful!
Preview URL: https://6ef7ff80.vald.pages.dev
Branch Preview URL: https://feature-lb-delete-with-times.vald.pages.dev

View logs

github-actions[bot]
github-actions bot reviewed Aug 25, 2023
View reviewed changes
pkg/agent/core/ngt/handler/grpc/remove_test.go Outdated Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/remove_test.go Outdated Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/remove_test.go Outdated Show resolved Hide resolved
@hlts2 hlts2 self-assigned this Aug 28, 2023
@hlts2 hlts2 force-pushed the feature/lb/delete-with-timestamp branch from 2698fe2 to 6786ee9 Compare August 28, 2023 03:56
github-actions[bot]
github-actions bot reviewed Aug 28, 2023
View reviewed changes
pkg/agent/core/ngt/handler/grpc/remove_test.go
@@ -366,6 +368,579 @@ func Test_server_Remove(t *testing.T) {
}
}

func Test_server_RemoveByTimestamp(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚫 [golangci] reported by reviewdog 🐶
Function name: Test_server_RemoveByTimestamp, Cyclomatic Complexity: 16, Halstead Volume: 5385.37, Maintainability Index: 19 (maintidx)

@github-actions
Copy link
Contributor

[FOSSA] The scan result will be available at https://app.fossa.com/projects/custom%2B21465%2Fvald/refs/branch/feature%2Flb%2Fdelete-with-timestamp/d66f6ed5c2ee969a1726b18db754e42014277814

@hlts2 hlts2 force-pushed the feature/lb/delete-with-timestamp branch from 2af923e to aaeac0b Compare August 31, 2023 01:43
@vankichi vankichi requested review from a team and kpango and removed request for kevindiu and a team August 31, 2023 04:42
@hlts2 hlts2 merged commit 2573b91 into main Aug 31, 2023
136 of 140 checks passed
@hlts2 hlts2 deleted the feature/lb/delete-with-timestamp branch August 31, 2023 05:26
@hlts2 hlts2 mentioned this pull request Sep 13, 2023
Merged
ykadowak added a commit that referenced this pull request Nov 30, 2023
@ykadowak
Revert "Add RemoveByTimestamp RPC (#2158)"
1261130 
This reverts commit 2573b91.
ykadowak added a commit that referenced this pull request Nov 30, 2023
@ykadowak
Revert "Add RemoveByTimestamp RPC (#2158)"
722f18d 
This reverts commit 2573b91.
ykadowak added a commit that referenced this pull request Nov 30, 2023
@ykadowak
Revert "Add RemoveByTimestamp RPC (#2158)"
0eb8d4e 
This reverts commit 2573b91.
ykadowak added a commit that referenced this pull request Nov 30, 2023
@ykadowak
Revert "Add RemoveByTimestamp RPC (#2158)"
893abbc 
This reverts commit 2573b91.
ykadowak added a commit that referenced this pull request Nov 30, 2023
@ykadowak
Revert "Add RemoveByTimestamp RPC (#2158)"
de92924 
This reverts commit 2573b91.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions GitHub Actions GitHub Actions left review comments

@kpango kpango kpango approved these changes

@datelier datelier datelier approved these changes

Assignees

@hlts2 hlts2

Labels
actions/e2e-chaos actions/e2e-deploy actions/e2e-max-dim actions/e2e-profiling actions/fossa area/agent/core area/agent/sidecar area/discoverer area/gateway/filter area/gateway/lb area/internal area/makefile area/manager/index priority/medium size/XXL team/set SET team team/sre SRE team type/feature New feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hlts2 @vdaas-ci @kpango @datelier