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

[agent-NGT] refactor pkg/agent/core/service using FOP #566

Conversation

rinx
Copy link
Contributor

@rinx rinx commented Jul 9, 2020

Description:

SSIA

further refactoring will be done with branch #556.

Related Issue:

nothing.

How Has This Been Tested?:

nothing

Environment:

  • Go Version: 1.14.3
  • Docker Version: 19.03.8
  • Kubernetes Version: 1.18.2
  • NGT Version: 1.11.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.

@pull-assistant
Copy link

pull-assistant bot commented Jul 9, 2020

Score: 1.00

Best reviewed: commit by commit


Optimal code review plan

     🔧 inject ngt service config from usecase layer

     ♻️ use filepath.Clean

     ✅ update tests

Powered by Pull Assistant. Last update 8cacdd5 ... 6a865c6. Read the comment docs.

@vdaas-ci
Copy link
Collaborator

vdaas-ci commented Jul 9, 2020

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 💌 /changelog - add changelog comment
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • /rebase - rebase master

@rinx rinx requested a review from kpango July 9, 2020 08:30
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.

others LGTM

Signed-off-by: Rintaro Okamura <[email protected]>
@codecov
Copy link

codecov bot commented Jul 9, 2020

Codecov Report

Merging #566 into refactor/agent-ngt-sidecar/improve-s3-backup-behavior will increase coverage by 0.28%.
The diff coverage is 0.00%.

Impacted file tree graph

@@                                    Coverage Diff                                    @@
##           refactor/agent-ngt-sidecar/improve-s3-backup-behavior     #566      +/-   ##
=========================================================================================
+ Coverage                                                  10.16%   10.44%   +0.28%     
=========================================================================================
  Files                                                        405      403       -2     
  Lines                                                      21007    20437     -570     
=========================================================================================
  Hits                                                        2135     2135              
+ Misses                                                     18612    18042     -570     
  Partials                                                     260      260              
Impacted Files Coverage Δ
pkg/agent/core/ngt/usecase/agentd.go 0.00% <0.00%> (ø)
pkg/agent/core/ngt/service/vcaches.go

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 1db65da...6a865c6. Read the comment docs.

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

Signed-off-by: Rintaro Okamura <[email protected]>
@github-actions github-actions bot added the team/set SET team label Jul 9, 2020
type args struct {
opts []core.Option
}
type fields struct {
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 🐶
struct of size 224 bytes could be of size 216 bytes (maligned)

type args struct {
ctx context.Context
}
type fields struct {
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 🐶
struct of size 224 bytes could be of size 216 bytes (maligned)

*/
}

for _, test := range tests {
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 🐶
copylocks: range var test copies lock: github.com/vdaas/vald/pkg/agent/core/ngt/service.test contains github.com/vdaas/vald/pkg/agent/core/ngt/service.fields contains sync.Mutex (govet)

ivc: test.fields.ivc,
dvc: test.fields.dvc,
indexing: test.fields.indexing,
saveMu: test.fields.saveMu,
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 🐶
copylocks: literal copies lock value from test.fields.saveMu: sync.Mutex (govet)

*/
}

for _, test := range tests {
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 🐶
copylocks: range var test copies lock: github.com/vdaas/vald/pkg/agent/core/ngt/service.test contains github.com/vdaas/vald/pkg/agent/core/ngt/service.fields contains sync.Mutex (govet)

ivc: test.fields.ivc,
dvc: test.fields.dvc,
indexing: test.fields.indexing,
saveMu: test.fields.saveMu,
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 🐶
copylocks: literal copies lock value from test.fields.saveMu: sync.Mutex (govet)

@rinx rinx merged commit 4f36cdd into refactor/agent-ngt-sidecar/improve-s3-backup-behavior Jul 9, 2020
@rinx rinx deleted the refactor/agent-ngt/ngt-service-fop branch July 9, 2020 08:52
rinx added a commit that referenced this pull request Jul 13, 2020
* 🔧 inject ngt service config from usecase layer

Signed-off-by: Rintaro Okamura <[email protected]>

* ♻️ use filepath.Clean

Signed-off-by: Rintaro Okamura <[email protected]>

* ✅ update tests

Signed-off-by: Rintaro Okamura <[email protected]>
vdaas-ci pushed a commit that referenced this pull request Jul 15, 2020
* 🔧 inject ngt service config from usecase layer

Signed-off-by: Rintaro Okamura <[email protected]>

* ♻️ use filepath.Clean

Signed-off-by: Rintaro Okamura <[email protected]>

* ✅ update tests

Signed-off-by: Rintaro Okamura <[email protected]>
vdaas-ci pushed a commit that referenced this pull request Jul 20, 2020
* 🔧 inject ngt service config from usecase layer

Signed-off-by: Rintaro Okamura <[email protected]>

* ♻️ use filepath.Clean

Signed-off-by: Rintaro Okamura <[email protected]>

* ✅ update tests

Signed-off-by: Rintaro Okamura <[email protected]>
vdaas-ci pushed a commit that referenced this pull request Jul 20, 2020
* 🔧 inject ngt service config from usecase layer

Signed-off-by: Rintaro Okamura <[email protected]>

* ♻️ use filepath.Clean

Signed-off-by: Rintaro Okamura <[email protected]>

* ✅ update tests

Signed-off-by: Rintaro Okamura <[email protected]>
kpango pushed a commit that referenced this pull request Jul 22, 2020
* ♻️ not to overwrite existing backup files

Signed-off-by: Rintaro Okamura <[email protected]>

* ✅ update tests for internal/compress

Signed-off-by: Rintaro Okamura <[email protected]>

* 🔧 add default_pool_size option / add internal/json package

Signed-off-by: Rintaro Okamura <[email protected]>

♻️ fix import

Signed-off-by: Rintaro Okamura <[email protected]>

* ✨ Add metadata package to write agent metadata when the index saved.

Signed-off-by: Rintaro Okamura <[email protected]>

* ✨ revise file.Open interface

Signed-off-by: Rintaro Okamura <[email protected]>

* ✅ fix file test

Signed-off-by: Rintaro Okamura <[email protected]>

* ✅ fix json tests

Signed-off-by: Rintaro Okamura <[email protected]>

* 🎨 use filepath.Join

Signed-off-by: Rintaro Okamura <[email protected]>

* ✨ revise watching method for ngt index backup files

Signed-off-by: Rintaro Okamura <[email protected]>

* ✅ remove dps from ngt_test

Signed-off-by: Rintaro Okamura <[email protected]>

* ✨ kill own process when NGT index cannot be loaded within timeout

Signed-off-by: Rintaro Okamura <[email protected]>

* 🚨 fix deepsource issues

Signed-off-by: Rintaro Okamura <[email protected]>

* 🚚 move internal/json -> internal/encoding/json

Signed-off-by: Rintaro Okamura <[email protected]>

* [agent-NGT] refactor pkg/agent/core/service using FOP (#566)

* 🔧 inject ngt service config from usecase layer

Signed-off-by: Rintaro Okamura <[email protected]>

* ♻️ use filepath.Clean

Signed-off-by: Rintaro Okamura <[email protected]>

* ✅ update tests

Signed-off-by: Rintaro Okamura <[email protected]>

* ♻️ refactor initNGT func

Signed-off-by: Rintaro Okamura <[email protected]>

* ✅ update

Signed-off-by: Rintaro Okamura <[email protected]>

* ✨ add isValid flag to metadata

Signed-off-by: Rintaro Okamura <[email protected]>

* 💚 revise fetch depth for run test workflow

Signed-off-by: Rintaro Okamura <[email protected]>

* ♻️ use isInvalid because it should be default to false

Signed-off-by: Rintaro Okamura <[email protected]>

* 🎨 fix tag format

Signed-off-by: Rintaro Okamura <[email protected]>

* ♻️ fix poststop logic

Signed-off-by: Rintaro Okamura <[email protected]>

* ✨ add Escape() method to errgroup

Signed-off-by: Rintaro Okamura <[email protected]>

* 📦 add *.containers field

Signed-off-by: Rintaro Okamura <[email protected]>

* Revert "📦 add *.containers field"

This reverts commit f82d52e.

* ✅ update

Signed-off-by: Rintaro Okamura <[email protected]>

* Revert "✅ update"

This reverts commit 1c6bce6.

* Revert ":sparkles: add Escape() method to errgroup"

This reverts commit 6a4dad6.

* ♻️ exit safely using goroutine

Signed-off-by: Rintaro Okamura <[email protected]>

* 🚚 move internal/metadata -> pkg/agent/internal/metadata

Signed-off-by: Rintaro Okamura <[email protected]>

* ♻️ add defaultPoolSize const

Signed-off-by: Rintaro Okamura <[email protected]>

* ♻️ pass cfg struct to agent service

Signed-off-by: Rintaro Okamura <[email protected]>

* 🐳 fix dockerfiles

Signed-off-by: Rintaro Okamura <[email protected]>

* ♻️ add watch_enabled & auto_backup_enabled options

Signed-off-by: Rintaro Okamura <[email protected]>

* ✅ fix tests

Signed-off-by: Rintaro Okamura <[email protected]>

* ♻️ revise agent-ngt codes based on suggestions

Signed-off-by: Rintaro Okamura <[email protected]>

* ✅ update

Signed-off-by: Rintaro Okamura <[email protected]>

* ♻️ revise observer poststop

Signed-off-by: Rintaro Okamura <[email protected]>
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.

3 participants