You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/11/17 16:20:24 UTC
[GitHub] [pinot] richardstartin opened a new pull request #7782: expose API with git branch and commit info
richardstartin opened a new pull request #7782:
URL: https://github.com/apache/pinot/pull/7782
This is added for benchmarking purposes, to make it easy to describe what's running in a benchmark.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#issuecomment-971767025
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7782](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b401e86) into [master](https://codecov.io/gh/apache/pinot/commit/d1606cd0f3877c1a8ec08255d003ef104a94c500?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d1606cd) will **decrease** coverage by `35.09%`.
> The diff coverage is `8.69%`.
[](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7782 +/- ##
=============================================
- Coverage 71.66% 36.57% -35.10%
+ Complexity 4083 80 -4003
=============================================
Files 1578 1578
Lines 80639 80664 +25
Branches 11986 11993 +7
=============================================
- Hits 57790 29500 -28290
- Misses 18956 48842 +29886
+ Partials 3893 2322 -1571
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `27.84% <8.69%> (+0.03%)` | :arrow_up: |
| unittests1 | `?` | |
| unittests2 | `14.59% <0.00%> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ler/api/resources/PinotVersionRestletResource.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VmVyc2lvblJlc3RsZXRSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...n/src/main/java/org/apache/pinot/common/Utils.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vVXRpbHMuamF2YQ==) | `43.07% <9.09%> (-19.72%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...a/org/apache/pinot/spi/config/table/TableType.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL1RhYmxlVHlwZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../org/apache/pinot/spi/data/DimensionFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9EaW1lbnNpb25GaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../org/apache/pinot/spi/data/readers/FileFormat.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9yZWFkZXJzL0ZpbGVGb3JtYXQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [932 more](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [d1606cd...b401e86](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#issuecomment-973296523
> I was thinking to provide a single api that has version and commit info in it, rather than two different apis. We can deprecate/remove the version api eventually. It can be a v2/version for example (or version/v2, whichever way we are versioning -- not sure).
Isn’t someone likely to be using the current API somewhere? I don’t like making backward incompatible changes. What is your concern with the approach taken, precisely?
The motivation for this addition is to be able to figure out which version of Pinot is running when benchmarking it, but I can do that other ways, so if the addition of this API is contentious, discussing it much further might outweigh the benefits.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#issuecomment-971767025
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7782](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (53fb3a0) into [master](https://codecov.io/gh/apache/pinot/commit/d1606cd0f3877c1a8ec08255d003ef104a94c500?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d1606cd) will **decrease** coverage by `57.08%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7782 +/- ##
=============================================
- Coverage 71.66% 14.58% -57.09%
+ Complexity 4083 80 -4003
=============================================
Files 1578 1533 -45
Lines 80639 78794 -1845
Branches 11986 11790 -196
=============================================
- Hits 57790 11489 -46301
- Misses 18956 66462 +47506
+ Partials 3893 843 -3050
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.58% <0.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/src/main/java/org/apache/pinot/common/Utils.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vVXRpbHMuamF2YQ==) | `0.00% <0.00%> (-62.80%)` | :arrow_down: |
| [...ler/api/resources/PinotVersionRestletResource.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VmVyc2lvblJlc3RsZXRSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1252 more](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [d1606cd...53fb3a0](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#issuecomment-973302722
> > I was thinking to provide a single api that has version and commit info in it, rather than two different apis. We can deprecate/remove the version api eventually. It can be a v2/version for example (or version/v2, whichever way we are versioning -- not sure).
>
> Isn’t someone likely to be using the current API somewhere? I don’t like making backward incompatible changes. What is your concern with the approach taken, precisely?
>
> The motivation for this addition is to be able to figure out which version of Pinot is running when benchmarking it, but I can do that other ways, so if the addition of this API is contentious, discussing it much further might outweigh the benefits.
My concern is api proliferation. If for whatever reason we want more info, then we will introduce a new api next time. Instead, if we introduce an expandable api now. Say, we return a json object with elements: version, git-commit, last-commit, etc. We can then add new json elements as needed and don't need to introduce newer API. We can deprecate the version api and remove it in a couple of releases.
The more apis we have, the more we need to worry about keeping things compatible forever (as we are doing now). Don't you agree?
I don't mean to block your progress. I think it is a useful API to have. Just introduce it as the new "version" api, and deprecate the old one, is all I am asking for. If you have a strong disagreement, then this is not worth debating about. But I don't want this to be held as a template for adding more apis each time we want something new.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#discussion_r752113345
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/Utils.java
##########
@@ -145,4 +149,36 @@ public static void logVersions() {
return componentVersions;
}
+
+ public static String getGitInfo() {
Review comment:
This follows the pattern of the version endpoint pretty much line for line. I even considered factoring out a common function to do the manifest scan but there are slight differences in logic so I decided against it.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#issuecomment-973306572
@mcvsubbu @richardstartin Since it almost follows the same way as getting the component versions, does it make sense to simply add 2 entries `branch` and `commit` into the version map? We don't need to change the existing rest API.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#issuecomment-973289605
> > Why not extend the /version URI to provide this information as opposed to introducing a new endpoint? I am also fine with /v2/version if it is not possible to make it backward compatible.
>
> This _does_ extend the /version URI: the URI is /version/commit to get the commit info.
> > Why not extend the /version URI to provide this information as opposed to introducing a new endpoint? I am also fine with /v2/version if it is not possible to make it backward compatible.
>
> This _does_ extend the /version URI: the URI is /version/commit to get the commit info.
I was thinking to provide a single api that has version and commit info in it, rather than two different apis. We can deprecate/remove the version api eventually. It can be a v2/version for example (or version/v2, whichever way we are versioning -- not sure).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#issuecomment-973305106
@mcvsubbu that makes sense, and I agree, proliferated APIs are hard to maintain and look messy. I will revisit this at a later date.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#discussion_r752185085
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/Utils.java
##########
@@ -145,4 +149,36 @@ public static void logVersions() {
return componentVersions;
}
+
+ public static String getGitInfo() {
+ // unless Utils was somehow loaded on the bootclasspath, this will not be null
+ // and will find all manifests
+ ClassLoader classLoader = Utils.class.getClassLoader();
+ if (classLoader != null) {
+ try {
+ Enumeration<URL> manifests = classLoader.getResources("META-INF/MANIFEST.MF");
+ while (manifests.hasMoreElements()) {
+ URL url = manifests.nextElement();
+ try (InputStream stream = url.openStream()) {
+ Manifest manifest = new Manifest(stream);
+ Attributes attributes = manifest.getMainAttributes();
+ if (attributes != null) {
+ String implementationTitle = attributes.getValue(Attributes.Name.IMPLEMENTATION_TITLE);
+ if (implementationTitle != null && implementationTitle.contains("pinot")) {
+ String branch = attributes.getValue(IMPLEMENTATION_BRANCH);
+ String gitCommitId = attributes.getValue(IMPLEMENTATION_COMMIT);
+ if (!Strings.isNullOrEmpty(branch) && !Strings.isNullOrEmpty(gitCommitId)) {
+ return branch + "/" + gitCommitId;
+ }
+ }
+ }
+ }
+ }
+ } catch (IOException e) {
+ // ignore
Review comment:
Ok I will address this if a compromise can otherwise be reached on the feature
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on a change in pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on a change in pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#discussion_r751806250
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/Utils.java
##########
@@ -145,4 +149,36 @@ public static void logVersions() {
return componentVersions;
}
+
+ public static String getGitInfo() {
Review comment:
why is this in pinot-common if this is a controller only API?Do you expect to extend the api to the other components as well?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jackjlli commented on a change in pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
jackjlli commented on a change in pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#discussion_r751693991
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/Utils.java
##########
@@ -145,4 +149,36 @@ public static void logVersions() {
return componentVersions;
}
+
+ public static String getGitInfo() {
+ // unless Utils was somehow loaded on the bootclasspath, this will not be null
+ // and will find all manifests
+ ClassLoader classLoader = Utils.class.getClassLoader();
+ if (classLoader != null) {
+ try {
+ Enumeration<URL> manifests = classLoader.getResources("META-INF/MANIFEST.MF");
+ while (manifests.hasMoreElements()) {
+ URL url = manifests.nextElement();
+ try (InputStream stream = url.openStream()) {
+ Manifest manifest = new Manifest(stream);
+ Attributes attributes = manifest.getMainAttributes();
+ if (attributes != null) {
+ String implementationTitle = attributes.getValue(Attributes.Name.IMPLEMENTATION_TITLE);
+ if (implementationTitle != null && implementationTitle.contains("pinot")) {
+ String branch = attributes.getValue(IMPLEMENTATION_BRANCH);
+ String gitCommitId = attributes.getValue(IMPLEMENTATION_COMMIT);
+ if (!Strings.isNullOrEmpty(branch) && !Strings.isNullOrEmpty(gitCommitId)) {
+ return branch + "/" + gitCommitId;
+ }
+ }
+ }
+ }
+ }
+ } catch (IOException e) {
+ // ignore
Review comment:
Will it be good to print the exception in the controller log?
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin closed pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
richardstartin closed pull request #7782:
URL: https://github.com/apache/pinot/pull/7782
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] mcvsubbu commented on pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
mcvsubbu commented on pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#issuecomment-973309329
that may be fine in principle, but it does seem like a hack. I think the current version api returns a map of library names and their versions. "branch" and "commit" will stand out as something not being library names.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#issuecomment-971767025
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7782](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b401e86) into [master](https://codecov.io/gh/apache/pinot/commit/d1606cd0f3877c1a8ec08255d003ef104a94c500?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d1606cd) will **decrease** coverage by `57.07%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7782 +/- ##
=============================================
- Coverage 71.66% 14.59% -57.08%
+ Complexity 4083 80 -4003
=============================================
Files 1578 1533 -45
Lines 80639 78794 -1845
Branches 11986 11790 -196
=============================================
- Hits 57790 11500 -46290
- Misses 18956 66455 +47499
+ Partials 3893 839 -3054
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.59% <0.00%> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...n/src/main/java/org/apache/pinot/common/Utils.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vVXRpbHMuamF2YQ==) | `0.00% <0.00%> (-62.80%)` | :arrow_down: |
| [...ler/api/resources/PinotVersionRestletResource.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29udHJvbGxlci9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29udHJvbGxlci9hcGkvcmVzb3VyY2VzL1Bpbm90VmVyc2lvblJlc3RsZXRSZXNvdXJjZS5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ain/java/org/apache/pinot/core/data/table/Key.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL0tleS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/spi/utils/BooleanUtils.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQm9vbGVhblV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/data/table/Record.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9kYXRhL3RhYmxlL1JlY29yZC5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [.../java/org/apache/pinot/core/util/GroupByUtils.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS91dGlsL0dyb3VwQnlVdGlscy5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/config/table/FSTType.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvY29uZmlnL3RhYmxlL0ZTVFR5cGUuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...ava/org/apache/pinot/spi/data/MetricFieldSpec.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvZGF0YS9NZXRyaWNGaWVsZFNwZWMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...va/org/apache/pinot/spi/utils/BigDecimalUtils.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3Qtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9zcGkvdXRpbHMvQmlnRGVjaW1hbFV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [...java/org/apache/pinot/common/tier/TierFactory.java](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdGllci9UaWVyRmFjdG9yeS5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| ... and [1252 more](https://codecov.io/gh/apache/pinot/pull/7782/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [d1606cd...b401e86](https://codecov.io/gh/apache/pinot/pull/7782?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
richardstartin commented on pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#issuecomment-972744488
> Why not extend the /version URI to provide this information as opposed to introducing a new endpoint? I am also fine with /v2/version if it is not possible to make it backward compatible.
This _does_ extend the /version URI: the URI is /version/commit to get the commit info.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] jackjlli commented on a change in pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
jackjlli commented on a change in pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#discussion_r751790345
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/Utils.java
##########
@@ -145,4 +149,36 @@ public static void logVersions() {
return componentVersions;
}
+
+ public static String getGitInfo() {
+ // unless Utils was somehow loaded on the bootclasspath, this will not be null
+ // and will find all manifests
+ ClassLoader classLoader = Utils.class.getClassLoader();
+ if (classLoader != null) {
+ try {
+ Enumeration<URL> manifests = classLoader.getResources("META-INF/MANIFEST.MF");
+ while (manifests.hasMoreElements()) {
+ URL url = manifests.nextElement();
+ try (InputStream stream = url.openStream()) {
+ Manifest manifest = new Manifest(stream);
+ Attributes attributes = manifest.getMainAttributes();
+ if (attributes != null) {
+ String implementationTitle = attributes.getValue(Attributes.Name.IMPLEMENTATION_TITLE);
+ if (implementationTitle != null && implementationTitle.contains("pinot")) {
+ String branch = attributes.getValue(IMPLEMENTATION_BRANCH);
+ String gitCommitId = attributes.getValue(IMPLEMENTATION_COMMIT);
+ if (!Strings.isNullOrEmpty(branch) && !Strings.isNullOrEmpty(gitCommitId)) {
+ return branch + "/" + gitCommitId;
+ }
+ }
+ }
+ }
+ }
+ } catch (IOException e) {
+ // ignore
Review comment:
Yeah I agree that the worst case is that commit hash won't be returned, but what I'm interested in here is to show any informative message for **pinot dev/ admin** to help debug the exception instead of silently swallowing it. We've already had the try catch blocks here, it'd be good to make use of it.
I'm fine with keeping it as is, just a good to have.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] klsince commented on a change in pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
klsince commented on a change in pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#discussion_r751450700
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/Utils.java
##########
@@ -145,4 +149,36 @@ public static void logVersions() {
return componentVersions;
}
+
+ public static String getGitInfo() {
+ // unless Utils was somehow loaded on the bootclasspath, this will not be null
+ // and will find all manifests
+ ClassLoader classLoader = Utils.class.getClassLoader();
+ if (classLoader != null) {
+ try {
+ Enumeration<URL> manifests = classLoader.getResources("META-INF/MANIFEST.MF");
+ while (manifests.hasMoreElements()) {
+ URL url = manifests.nextElement();
+ try (InputStream stream = url.openStream()) {
+ Manifest manifest = new Manifest(stream);
+ Attributes attributes = manifest.getMainAttributes();
+ if (attributes != null) {
+ String implementationTitle = attributes.getValue(Attributes.Name.IMPLEMENTATION_TITLE);
Review comment:
nit: leave a comment that those info are injected via configs in that pom.xml file
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] richardstartin commented on a change in pull request #7782: expose API with git branch and commit info
Posted by GitBox <gi...@apache.org>.
richardstartin commented on a change in pull request #7782:
URL: https://github.com/apache/pinot/pull/7782#discussion_r751783181
##########
File path: pinot-common/src/main/java/org/apache/pinot/common/Utils.java
##########
@@ -145,4 +149,36 @@ public static void logVersions() {
return componentVersions;
}
+
+ public static String getGitInfo() {
+ // unless Utils was somehow loaded on the bootclasspath, this will not be null
+ // and will find all manifests
+ ClassLoader classLoader = Utils.class.getClassLoader();
+ if (classLoader != null) {
+ try {
+ Enumeration<URL> manifests = classLoader.getResources("META-INF/MANIFEST.MF");
+ while (manifests.hasMoreElements()) {
+ URL url = manifests.nextElement();
+ try (InputStream stream = url.openStream()) {
+ Manifest manifest = new Manifest(stream);
+ Attributes attributes = manifest.getMainAttributes();
+ if (attributes != null) {
+ String implementationTitle = attributes.getValue(Attributes.Name.IMPLEMENTATION_TITLE);
+ if (implementationTitle != null && implementationTitle.contains("pinot")) {
+ String branch = attributes.getValue(IMPLEMENTATION_BRANCH);
+ String gitCommitId = attributes.getValue(IMPLEMENTATION_COMMIT);
+ if (!Strings.isNullOrEmpty(branch) && !Strings.isNullOrEmpty(gitCommitId)) {
+ return branch + "/" + gitCommitId;
+ }
+ }
+ }
+ }
+ }
+ } catch (IOException e) {
+ // ignore
Review comment:
The /version endpoint doesn’t log either. If this endpoint fails, the worst thing that’s going to happen is the commit hash won’t be returned, it’s a purely informational endpoint, like /version.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org