Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Split agent pkg handler implementation #1690

Merged
merged 6 commits into from
Jun 8, 2022
Merged

Split agent pkg handler implementation #1690

merged 6 commits into from
Jun 8, 2022

Conversation

kevindiu
Copy link
Contributor

@kevindiu kevindiu commented Jun 6, 2022 •
edited
Loading

Description:

This PR implements the following discussion to split pkg/agent/core/ngt/handler/grpc/handler.go and corresponding test implementation.

#1683

Related Issue:

How Has This Been Tested?:

Environment:

  • Go Version: 1.18.2
  • Docker Version: 20.10.8
  • Kubernetes Version: 1.22.0
  • NGT Version: 1.14.5

Types of changes:

  • Bug fix [type/bug]
  • New feature [type/feature]
  • Add tests [type/test]
  • Security related changes [type/security]
  • Add documents [type/documentation]
  • Refactoring [type/refactoring]
  • Update dependencies [type/dependency]
  • Update benchmarks and performances [type/bench]
  • Update CI [type/ci]

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Checklist:

  • I have read the CONTRIBUTING document.
  • I have checked open Pull Requests for the similar feature or fixes?
  • I have added tests and benchmarks to cover my changes.
  • I have ensured all new and existing tests passed.
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly.

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jun 6, 2022

[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 master
  • 🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

@codecov
Copy link

codecov bot commented Jun 6, 2022 •
edited
Loading

Codecov Report

Merging #1690 (8363790) into master (39187c5) will increase coverage by 0.04%.
The diff coverage is 19.72%.

@@            Coverage Diff             @@
##           master    #1690      +/-   ##
==========================================
+ Coverage   31.22%   31.27%   +0.04%     
==========================================
  Files         377      385       +8     
  Lines       32679    32679              
==========================================
+ Hits        10204    10219      +15     
+ Misses      22086    22076      -10     
+ Partials      389      384       -5
Impacted Files Coverage Δ
pkg/agent/core/ngt/handler/grpc/handler.go 71.42% <ø> (+50.86%) ⬆️
pkg/agent/core/ngt/handler/grpc/linear_search.go 0.00% <0.00%> (ø)
pkg/agent/core/ngt/handler/grpc/upsert.go 0.00% <0.00%> (ø)
pkg/agent/core/ngt/handler/grpc/index.go 4.91% <4.91%> (ø)
pkg/agent/core/ngt/handler/grpc/remove.go 22.27% <22.27%> (ø)
pkg/agent/core/ngt/handler/grpc/search.go 24.12% <24.12%> (ø)
pkg/agent/core/ngt/handler/grpc/update.go 27.40% <27.40%> (ø)
pkg/agent/core/ngt/handler/grpc/object.go 30.28% <30.28%> (ø)
pkg/agent/core/ngt/handler/grpc/insert.go 61.77% <61.77%> (ø)
internal/errgroup/group.go 94.00% <0.00%> (-1.00%) ⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7ed006d...8363790. Read the comment docs.

@kevindiu kevindiu marked this pull request as ready for review June 6, 2022 02:11
@kevindiu kevindiu changed the title [WIP] Split agent pkg handler implementation Split agent pkg handler implementation Jun 6, 2022
@kevindiu kevindiu requested review from kmrmt and vankichi June 6, 2022 02:11
github-actions[bot]
github-actions bot reviewed Jun 6, 2022
View reviewed changes
pkg/agent/core/ngt/handler/grpc/insert.go Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/object.go Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/remove.go Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/update.go Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/upsert.go Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/object_test.go Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/remove_test.go Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/search_test.go Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/search_test.go Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/update_test.go Show resolved Hide resolved
@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jun 6, 2022

Profile Report

typevald-agent-ngtvald-lb-gatewayvald-discoverervald-manager-index
cpu
heap
 other images

@kevindiu kevindiu force-pushed the refactor/agent/split-pkg-handler-impl branch 2 times, most recently from 33a85ea to afcd3ff Compare June 6, 2022 05:54
github-actions[bot]
github-actions bot reviewed Jun 6, 2022
View reviewed changes
pkg/agent/core/ngt/handler/grpc/update.go Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/insert_test.go Outdated Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/insert_test.go Outdated Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/object_test.go Outdated Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/object_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
pkg/agent/core/ngt/handler/grpc/search_test.go Outdated Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/search_test.go Outdated Show resolved Hide resolved
pkg/agent/core/ngt/handler/grpc/update_test.go Outdated Show resolved Hide resolved
@kevindiu kevindiu force-pushed the refactor/agent/split-pkg-handler-impl branch from afcd3ff to 05e1dfe Compare June 6, 2022 08:19
hlts2
hlts2 previously approved these changes Jun 7, 2022
View reviewed changes
Copy link
Collaborator

@hlts2 hlts2 left a comment

Choose a reason for hiding this comment

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

LGTM

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jun 7, 2022

Profile Report

typevald-agent-ngtvald-lb-gatewayvald-discoverervald-manager-index
cpu
heap
 other images

@kevindiu kevindiu force-pushed the refactor/agent/split-pkg-handler-impl branch from 5fcb51d to 325d5a3 Compare June 7, 2022 08:14
@hlts2 hlts2 dismissed stale reviews from vankichi and themself via 325d5a3 June 7, 2022 09:09
kevindiu added 6 commits June 7, 2022 18:15
@kevindiu
split handler impl code
4230b4f 
Signed-off-by: kevindiu <kevindiujp@gmail.com>
@kevindiu
split handler test code
464ce49 
Signed-off-by: kevindiu <kevindiujp@gmail.com>
@kevindiu
add license header
57bc4b2 
Signed-off-by: kevindiu <kevindiujp@gmail.com>
@kevindiu
fix ci warning
717818e 
Signed-off-by: kevindiu <kevindiujp@gmail.com>
@kevindiu
fix ci warning
c281131 
Signed-off-by: kevindiu <kevindiujp@gmail.com>
@kevindiu
fix trace span name
8363790 
Signed-off-by: kevindiu <kevindiujp@gmail.com>
@kevindiu kevindiu force-pushed the refactor/agent/split-pkg-handler-impl branch from 325d5a3 to 8363790 Compare June 7, 2022 09:15
hlts2
hlts2 approved these changes Jun 7, 2022
View reviewed changes
Copy link
Collaborator

@hlts2 hlts2 left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link
Contributor

github-actions bot commented Jun 7, 2022

[FOSSA] The scan result will be available at https://app.fossa.com/projects/custom%2B21465%2Fvald/refs/branch/refactor%2Fagent%2Fsplit-pkg-handler-impl/aa91ff7b8d7dd306e105fe1fba3a8f22a50d9b41

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jun 7, 2022

Profile Report

typevald-agent-ngtvald-lb-gatewayvald-discoverervald-manager-index
cpu
heap
 other images

@kevindiu kevindiu merged commit 35eba88 into master Jun 8, 2022
@kevindiu kevindiu deleted the refactor/agent/split-pkg-handler-impl branch June 8, 2022 01:13
@hlts2 hlts2 mentioned this pull request Jun 8, 2022
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@kpango kpango kpango left review comments

@hlts2 hlts2 hlts2 approved these changes

@vankichi vankichi vankichi approved these changes

@kmrmt kmrmt kmrmt approved these changes

Assignees
No one assigned
Labels
actions/e2e-chaos actions/e2e-deploy actions/e2e-max-dim actions/e2e-profiling actions/fossa area/agent/core priority/low size/XXXL team/set SET team type/refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@kevindiu @vdaas-ci @kmrmt @kpango @vankichi @hlts2