Use go-events package #2550
Use go-events package #2550
Conversation
d74d90b to
8cd82db
Compare
Codecov Report
@@ Coverage Diff @@
## master #2550 +/- ##
==========================================
- Coverage 58.67% 58.26% -0.42%
==========================================
Files 104 104
Lines 8434 8341 -93
==========================================
- Hits 4949 4860 -89
+ Misses 2823 2822 -1
+ Partials 662 659 -3
Continue to review full report at Codecov.
|
|
See also #2551 and docker/go-events#24. |
notifications/http_test.go
Outdated
| @@ -51,7 +52,7 @@ func TestHTTPSink(t *testing.T) { | |||
| } | |||
|
|
|||
| // Let caller choose the status | |||
| status, err := strconv.Atoi(r.FormValue("status")) | |||
| status, err := strconv.Atoi(r.URL.Query().Get("status")) | |||
There was a problem hiding this comment.
Why is this change made? r.FormValue is equivalent, no?
There was a problem hiding this comment.
This was failing, looks like due to a mistake I had made during the conversion, so I've set it back now.
| @@ -61,7 +63,7 @@ func NewEndpoint(name, url string, config EndpointConfig) *Endpoint { | |||
| endpoint.Sink = newHTTPSink( | |||
| endpoint.url, endpoint.Timeout, endpoint.Headers, | |||
| endpoint.Transport, endpoint.metrics.httpStatusListener()) | |||
| endpoint.Sink = newRetryingSink(endpoint.Sink, endpoint.Threshold, endpoint.Backoff) | |||
| endpoint.Sink = events.NewRetryingSink(endpoint.Sink, events.NewBreaker(endpoint.Threshold, endpoint.Backoff)) | |||
|
cc @dmcgowan |
8cd82db to
ec7781d
Compare
|
Updated to resolve merge conflicts. |
notifications/metrics.go
Outdated
| emsl.Statuses[fmt.Sprintf("%d %s", status, http.StatusText(status))] += len(events) | ||
| emsl.Successes += len(events) | ||
| evt, ok := event.(Event) | ||
| if !ok || (ok && evt.ID == "") { |
There was a problem hiding this comment.
Does this happen? If we are seeing this, there might be a bug.
There was a problem hiding this comment.
I believe this shouldn't happen. From memory I think I added this after I accidentally sent the wrong thing through here partway through converting things. I'll remove it.
2c50c1d to
bbb1e75
Compare
|
LGTM I think the tests are failing with a concurrency issue when setting the url on the sink for each test case: https://github.com/docker/distribution/pull/2550/files#diff-e8f917a7e7a5472ef1ca0e58b0f7775aL162. |
14e3fd8 to
08e265b
Compare
| @@ -64,7 +65,7 @@ func NewEndpoint(name, url string, config EndpointConfig) *Endpoint { | |||
| endpoint.Sink = newHTTPSink( | |||
| endpoint.url, endpoint.Timeout, endpoint.Headers, | |||
| endpoint.Transport, endpoint.metrics.httpStatusListener()) | |||
| endpoint.Sink = newRetryingSink(endpoint.Sink, endpoint.Threshold, endpoint.Backoff) | |||
| endpoint.Sink = events.NewRetryingSink(endpoint.Sink, events.NewBreaker(endpoint.Threshold, endpoint.Backoff)) | |||
| endpoint.Sink = newEventQueue(endpoint.Sink, endpoint.metrics.eventQueueListener()) | |||
There was a problem hiding this comment.
Will be nice to have this also in go-events but can be done later.
|
|
||
| statusCounter.WithValues(fmt.Sprintf("%d %s", status, http.StatusText(status)), emsl.EndpointName).Inc(1) | ||
| eventsCounter.WithValues("Successes", emsl.EndpointName).Inc(float64(len(events))) | ||
| eventsCounter.WithValues("Successes", emsl.EndpointName).Inc(1) |
There was a problem hiding this comment.
Nice. Had fixed this bug earlier but didn't upstream it.
| } | ||
| if len(kept) == 0 { | ||
| func (imts *ignoredSink) Write(event events.Event) error { | ||
| if imts.ignoreMediaTypes[event.(Event).Target.MediaType] || imts.ignoreActions[event.(Event).Action] { |
TBD: Queue not converted yet Signed-off-by: Elliot Pahl <elliot.pahl@gmail.com>
08e265b to
800cb95
Compare
This addresses #1553.
TBD: Queue not converted yet
Signed-off-by: Elliot Pahl elliot.pahl@gmail.com