vbehar commented 5 years ago

The first implementation of the per-deployment metricsCheckInterval never took into account updates of the annotation value. The new one read the annotation value and compare it to the current one to decide if the metrics collector should be recreated or not.

codecov-io commented 5 years ago

Codecov Report

Merging #52 into master will decrease coverage by 18.76%. The diff coverage is 87.17%.

@@             Coverage Diff             @@
##           master      #52       +/-   ##
===========================================
- Coverage   59.09%   40.32%   -18.77%     
===========================================
  Files          11       14        +3     
  Lines         638      977      +339     
===========================================
+ Hits          377      394       +17     
- Misses        232      554      +322     
  Partials       29       29

Impacted Files	Coverage Δ
pkg/kubernetes/osiris.go	`100% <ø> (ø)`	:arrow_up:
pkg/deployments/zeroscaler/zeroscaler.go	`21.65% <87.17%> (ø)`
pkg/net/http/httputil/reverseproxy.go	`72.22% <0%> (-0.8%)`	:arrow_down:
pkg/deployments/zeroscaler/metrics_collector.go	`0% <0%> (ø)`
pkg/deployments/zeroscaler/config.go	`0% <0%> (ø)`

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 a197da3...a10513e. Read the comment docs.

krancour commented 5 years ago

@vbehar, registered one nit, but overall this is great. Thanks.

krancour commented 4 years ago

@vbehar I apologize for dropping the ball on this (and some of your other PRs as well). This LGTM and I will merge pending completion of the current CI run.

deislabs / osiris

Ensure updates to metricsCheckInterval are taken into account #52

Codecov Report