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

refactor: cleanup and move API #95

Merged
merged 88 commits into from
Feb 8, 2024
Merged

refactor: cleanup and move API #95

merged 88 commits into from
Feb 8, 2024

Conversation

lvlcn-t
Copy link
Member

@lvlcn-t lvlcn-t commented Jan 30, 2024 •
edited
Loading

Motivation

To finish the refactoring started in #91, where the checks' handler methods were removed from the interface.

Closes #92

Changes

  • I've refactored the API to a dedicated component in another package behind an interface. We partly discussed this here.
  • I've refactored the Sparrow struct into a composition structure in order to improve code organization and clarity. This includes setting apart different responsibilities into checkCoordinator and errorHandler types.
  • I've removed functionalities and sections of code that were no longer needed.

(I also adjusted the docs of one dns metric that has changed)

Tests done

  • I've moved/refactored the unit tests for the API
  • I've added some more unit tests
  • Unit tests suceeded.
  • E2E tests succeeded.

TODO

  • I've assigned this PR to myself
  • I've labeled this PR correctly

lvlcn-t and others added 30 commits January 14, 2024 20:00
@lvlcn-t
refactor: mv api and metrics to dedicated packages
55616be
@niklastreml
chore: updated helm chart version to 0.0.4
6f759d9
@niklastreml
chore: bump appVersion to v0.3.0
820cd55
@lvlcn-t
fix: incorrect formatting on empty labels
da317cc
@puffitos
refactor: remove any configurations
02e06b5 
Signed-off-by: Bruno Bressi <[email protected]>
@puffitos
chore: removed decode (RIP)
078de7c 
Signed-off-by: Bruno Bressi <[email protected]>
@lvlcn-t
fix: newline indent
975e77f
@puffitos
chore: merged from main
4a2d58f 
Signed-off-by: Bruno Bressi <[email protected]>
@puffitos
refactor: restructure checks pkg
68d066a 
Signed-off-by: Bruno Bressi <[email protected]>
@niklastreml
fix: remove default runtime config in helm chart
761d7c4
@niklastreml
feat: simplify helm chart
2fa7fb2
@niklastreml
fix: remove -n
b9abd15
@niklastreml
fix: register healthcheck for e2e test
8bb3f07
@niklastreml
feat: separate checks and sparrow config
c8c764a
@niklastreml
fix: e2e tests
8f0d564
@niklastreml
fix: bug where env variables would not be read correctly
551f07c
@niklastreml
feat: allow mounting extra secrets as env variables
d81ad4a
@puffitos
Merge remote-tracking branch 'origin/feat/simplify-helm-chart-config'…
3f40604 
… into refactor/typed-runtime-cfg
@puffitos
Merge remote-tracking branch 'origin' into refactor/typed-runtime-cfg
7fa28db
@niklastreml
Merge branch 'main' into chore/bump-chart-version
fc72395
@niklastreml
Merge branch 'chore/bump-chart-version' into feat/simplify-helm-chart…
2a1cb4c 
…-config
@puffitos
chore: added helm docs
0bd9674 
Signed-off-by: Bruno Bressi <[email protected]>
@puffitos
Merge branch 'feat/simplify-helm-chart-config' of github.com:caas-tea…
c6f0ca7 
…m/sparrow into feat/simplify-helm-chart-config
@niklastreml
docs: envfromSecrets
0c4042f
@puffitos
chore: merge from main
9339cdd 
Signed-off-by: Bruno Bressi <[email protected]>
@puffitos
Merge branch 'feat/simplify-helm-chart-config' into refactor/typed-ru…
90988e7 
…ntime-cfg
@puffitos
fix: checks are properly configured on start
3d0971f 
Signed-off-by: Bruno Bressi <[email protected]>
@puffitos
fix: target manager nil check when enriching
2f6333b 
Signed-off-by: Bruno Bressi <[email protected]>
@puffitos
chore: merge from main
eb5fc0a 
Signed-off-by: Bruno Bressi <[email protected]>
@puffitos
Merge remote-tracking branch 'origin/main' into refactor/typed-runtim…
2742590 
…e-cfg
@lvlcn-t lvlcn-t added refactoring Refactoring of existing code housekeeping area/api Issues/PRs related to the API labels Jan 30, 2024
@lvlcn-t lvlcn-t added this to the 0.4.0 milestone Jan 30, 2024
@lvlcn-t lvlcn-t self-assigned this Jan 30, 2024
lvlcn-t
lvlcn-t commented Jan 30, 2024
View reviewed changes
pkg/api/api.go Show resolved Hide resolved
puffitos
puffitos requested changes Feb 2, 2024
View reviewed changes
Copy link
Member

@puffitos puffitos left a comment

Choose a reason for hiding this comment

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

All in all, everything seems fine. The Run method of the sparrow has to change, to keep the focus on its responsibility.

pkg/api/api.go Outdated Show resolved Hide resolved
pkg/sparrow/run.go Outdated Show resolved Hide resolved
pkg/sparrow/run.go Show resolved Hide resolved
pkg/sparrow/run.go Show resolved Hide resolved
pkg/api/api.go Show resolved Hide resolved
pkg/api/api.go Outdated Show resolved Hide resolved
niklastreml
niklastreml reviewed Feb 7, 2024
View reviewed changes
pkg/api/api.go Outdated Show resolved Hide resolved
pkg/api/api.go Outdated Show resolved Hide resolved
pkg/api/api.go Show resolved Hide resolved
@lvlcn-t lvlcn-t merged commit ea6fdca into main Feb 8, 2024
11 checks passed
@lvlcn-t lvlcn-t deleted the refactor/api branch February 8, 2024 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@puffitos puffitos puffitos approved these changes

@niklastreml niklastreml niklastreml approved these changes

@y-eight y-eight Awaiting requested review from y-eight y-eight is a code owner

@eumel8 eumel8 Awaiting requested review from eumel8

Assignees

@lvlcn-t lvlcn-t

Labels
area/api Issues/PRs related to the API housekeeping refactoring Refactoring of existing code
Projects
None yet
Milestone
0.4.0
Development

Successfully merging this pull request may close these issues.

Cleanup API with dynamic Handler
3 participants
@lvlcn-t @niklastreml @puffitos