You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by GitBox <gi...@apache.org> on 2020/08/27 04:06:00 UTC
[GitHub] [incubator-yunikorn-core] yangwwei opened a new pull request #200: [YUNIKORN-375] Use compatible Prometheus client library
yangwwei opened a new pull request #200:
URL: https://github.com/apache/incubator-yunikorn-core/pull/200
Better to use a more stable Prometheus version, this PR is based on
```
go get github.com/prometheus/client_golang@v0.9.2
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #200: [YUNIKORN-375] Use compatible Prometheus client library
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #200:
URL: https://github.com/apache/incubator-yunikorn-core/pull/200#issuecomment-681367815
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/200?src=pr&el=h1) Report
> Merging [#200](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/200?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/55192588b53ec02e8397ec1d7f7632af5938a374?el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/200?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #200 +/- ##
=======================================
Coverage 60.61% 60.61%
=======================================
Files 67 67
Lines 6533 6533
=======================================
Hits 3960 3960
Misses 2427 2427
Partials 146 146
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/200?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/200?src=pr&el=footer). Last update [5519258...809813b](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/200?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] wilfred-s closed pull request #200: [YUNIKORN-375] Use compatible Prometheus client library
Posted by GitBox <gi...@apache.org>.
wilfred-s closed pull request #200:
URL: https://github.com/apache/incubator-yunikorn-core/pull/200
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] commented on pull request #200: [YUNIKORN-375] Use compatible Prometheus client library
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #200:
URL: https://github.com/apache/incubator-yunikorn-core/pull/200#issuecomment-681367815
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/200?src=pr&el=h1) Report
> Merging [#200](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/200?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/55192588b53ec02e8397ec1d7f7632af5938a374?el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/200?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #200 +/- ##
=======================================
Coverage 60.61% 60.61%
=======================================
Files 67 67
Lines 6533 6533
=======================================
Hits 3960 3960
Misses 2427 2427
Partials 146 146
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/200?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/200?src=pr&el=footer). Last update [5519258...9f398a2](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/200?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] yangwwei commented on a change in pull request #200: [YUNIKORN-375] Use compatible Prometheus client library
Posted by GitBox <gi...@apache.org>.
yangwwei commented on a change in pull request #200:
URL: https://github.com/apache/incubator-yunikorn-core/pull/200#discussion_r478132551
##########
File path: go.mod
##########
@@ -23,15 +23,17 @@ go 1.12
require (
github.com/apache/incubator-yunikorn-scheduler-interface v0.9.1-0.20200827013520-ec2f681ecb5b
+ github.com/beorn7/perks v1.0.1 // indirect
github.com/golang/protobuf v1.4.0 // indirect
github.com/gorilla/mux v1.7.3
github.com/kr/text v0.2.0 // indirect
github.com/looplab/fsm v0.1.0
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
github.com/pkg/errors v0.9.1 // indirect
- github.com/prometheus/client_golang v1.4.0
+ github.com/prometheus/client_golang v0.9.2
Review comment:
Hi @sunilgovind @sunilgovind the reason to use 0.9.2 is that this is the version used by k8s 1.16. Just want to keep the consistency because we will need to push metrics to the compatible Prometheus that works with 1.16. We are not using new APIs so I guess we are fine with downgrade. What do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] sunilgovind commented on a change in pull request #200: [YUNIKORN-375] Use compatible Prometheus client library
Posted by GitBox <gi...@apache.org>.
sunilgovind commented on a change in pull request #200:
URL: https://github.com/apache/incubator-yunikorn-core/pull/200#discussion_r478105547
##########
File path: go.mod
##########
@@ -23,15 +23,17 @@ go 1.12
require (
github.com/apache/incubator-yunikorn-scheduler-interface v0.9.1-0.20200827013520-ec2f681ecb5b
+ github.com/beorn7/perks v1.0.1 // indirect
github.com/golang/protobuf v1.4.0 // indirect
github.com/gorilla/mux v1.7.3
github.com/kr/text v0.2.0 // indirect
github.com/looplab/fsm v0.1.0
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
github.com/pkg/errors v0.9.1 // indirect
- github.com/prometheus/client_golang v1.4.0
+ github.com/prometheus/client_golang v0.9.2
Review comment:
@yangwwei why are we bumping down the version from a stable 1.x to a 0.x version? if there are compatibility issues, it may get missed behind the scenes.
How could we verify that we dont have a breakage?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] TravisBuddy commented on pull request #200: [YUNIKORN-375] Use compatible Prometheus client library
Posted by GitBox <gi...@apache.org>.
TravisBuddy commented on pull request #200:
URL: https://github.com/apache/incubator-yunikorn-core/pull/200#issuecomment-681569486
Hey @yangwwei,
Your changes look good to me!
<a href="https://travis-ci.org/apache/incubator-yunikorn-core/builds/721583502">View build log</a>
###### TravisBuddy Request Identifier: 408606d0-e829-11ea-a3ad-29627dfdd765
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] wilfred-s commented on a change in pull request #200: [YUNIKORN-375] Use compatible Prometheus client library
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #200:
URL: https://github.com/apache/incubator-yunikorn-core/pull/200#discussion_r478111135
##########
File path: go.mod
##########
@@ -23,15 +23,17 @@ go 1.12
require (
github.com/apache/incubator-yunikorn-scheduler-interface v0.9.1-0.20200827013520-ec2f681ecb5b
+ github.com/beorn7/perks v1.0.1 // indirect
github.com/golang/protobuf v1.4.0 // indirect
github.com/gorilla/mux v1.7.3
github.com/kr/text v0.2.0 // indirect
github.com/looplab/fsm v0.1.0
github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
github.com/pkg/errors v0.9.1 // indirect
- github.com/prometheus/client_golang v1.4.0
+ github.com/prometheus/client_golang v0.9.2
Review comment:
That is exactly what I was looking at. 0.9.2 is not the latest on the 0.9.x branch either, 0.9.4 is.
We are downgrading to an almost 2 year old release which has had two updates on top.
I can understand that we might need to stay at a pre 1.0 release due to breaking changes and k8s or etcd lagging behind but we should be at least at the latest 0.9.x release.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] TravisBuddy commented on pull request #200: [YUNIKORN-375] Use compatible Prometheus client library
Posted by GitBox <gi...@apache.org>.
TravisBuddy commented on pull request #200:
URL: https://github.com/apache/incubator-yunikorn-core/pull/200#issuecomment-681367676
Hey @yangwwei,
Your changes look good to me!
<a href="https://travis-ci.org/apache/incubator-yunikorn-core/builds/721565587">View build log</a>
###### TravisBuddy Request Identifier: 1a5a54b0-e81b-11ea-a3ad-29627dfdd765
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org