You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/01/02 08:29:39 UTC
[GitHub] [skywalking] web-xiaxia opened a new pull request #4161: add
Dynamic Configuration Consul AclToken
web-xiaxia opened a new pull request #4161: add Dynamic Configuration Consul AclToken
URL: https://github.com/apache/skywalking/pull/4161
Please answer these questions before submitting pull request
- Why submit this pull request?
- [ ] Bug fix
- [x] New feature provided
- [ ] Improve performance
- Related issues
___
### Bug fix
- Bug description.
- How to fix?
___
### New feature or improvement
- Describe the details and related test reports.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4161:
add Dynamic Configuration Consul AclToken
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4161: add Dynamic Configuration Consul AclToken
URL: https://github.com/apache/skywalking/pull/4161#discussion_r362402954
##########
File path: oap-server/server-bootstrap/src/main/resources/application.yml
##########
@@ -223,6 +225,8 @@ configuration:
# hostAndPorts: ${consul.address}
# # Sync period in seconds. Defaults to 60 seconds.
# period: 1
+# # Consul aclToken
+# #aclToken: ${consul.aclToken}
Review comment:
If you want this too, you miss some codes change in the configuration model. Your changes are only related to cluster model.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4161:
add Dynamic Configuration Consul AclToken
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4161: add Dynamic Configuration Consul AclToken
URL: https://github.com/apache/skywalking/pull/4161#discussion_r362425786
##########
File path: oap-server/server-cluster-plugin/cluster-consul-plugin/src/main/java/org/apache/skywalking/oap/server/cluster/plugin/consul/ClusterModuleConsulProvider.java
##########
@@ -76,6 +76,10 @@ public ClusterModuleConsulProvider() {
// we should set this value or it will be blocked forever
.withConnectTimeoutMillis(3000);
+ if (config.getAclToken() != null && !config.getAclToken().isEmpty()) {
Review comment:
@web-xiaxia this should be fixed
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io edited a comment on issue #4161: add
Dynamic Configuration Consul AclToken
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4161: add Dynamic Configuration Consul AclToken
URL: https://github.com/apache/skywalking/pull/4161#issuecomment-570151024
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=h1) Report
> Merging [#4161](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/568c2e53f09855199884f65f38a9e4771a5a6467?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `0%`.
[](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4161 +/- ##
==========================================
- Coverage 26.89% 26.86% -0.03%
==========================================
Files 1160 1160
Lines 25377 25381 +4
Branches 3680 3682 +2
==========================================
- Hits 6824 6818 -6
- Misses 17945 17955 +10
Partials 608 608
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ter/plugin/consul/ClusterModuleConsulProvider.java](https://codecov.io/gh/apache/skywalking/pull/4161/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY2x1c3Rlci1wbHVnaW4vY2x1c3Rlci1jb25zdWwtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY2x1c3Rlci9wbHVnaW4vY29uc3VsL0NsdXN0ZXJNb2R1bGVDb25zdWxQcm92aWRlci5qYXZh) | `92.85% <0%> (-7.15%)` | :arrow_down: |
| [...ion/consul/ConsulConfigurationWatcherRegister.java](https://codecov.io/gh/apache/skywalking/pull/4161/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29uZmlndXJhdGlvbi9jb25maWd1cmF0aW9uLWNvbnN1bC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvbmZpZ3VyYXRpb24vY29uc3VsL0NvbnN1bENvbmZpZ3VyYXRpb25XYXRjaGVyUmVnaXN0ZXIuamF2YQ==) | `51.92% <0%> (-2.08%)` | :arrow_down: |
| [...apm/agent/core/remote/GRPCStreamServiceStatus.java](https://codecov.io/gh/apache/skywalking/pull/4161/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENTdHJlYW1TZXJ2aWNlU3RhdHVzLmphdmE=) | `29.16% <0%> (-25.01%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4161?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/skywalking/pull/4161?src=pr&el=footer). Last update [568c2e5...aff505b](https://codecov.io/gh/apache/skywalking/pull/4161?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
With regards,
Apache Git Services
[GitHub] [skywalking] dmsolr merged pull request #4161: add Dynamic
Configuration Consul AclToken
Posted by GitBox <gi...@apache.org>.
dmsolr merged pull request #4161: add Dynamic Configuration Consul AclToken
URL: https://github.com/apache/skywalking/pull/4161
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] dmsolr commented on a change in pull request #4161:
add Dynamic Configuration Consul AclToken
Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #4161: add Dynamic Configuration Consul AclToken
URL: https://github.com/apache/skywalking/pull/4161#discussion_r362413572
##########
File path: oap-server/server-cluster-plugin/cluster-consul-plugin/src/main/java/org/apache/skywalking/oap/server/cluster/plugin/consul/ClusterModuleConsulProvider.java
##########
@@ -76,6 +76,10 @@ public ClusterModuleConsulProvider() {
// we should set this value or it will be blocked forever
.withConnectTimeoutMillis(3000);
+ if (config.getAclToken() != null && !config.getAclToken().isEmpty()) {
Review comment:
Why do not you use `!Strings.isNullOrEmpty(config.getAclToken())`? I think that it is more readable.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] web-xiaxia commented on issue #4161: add Dynamic
Configuration Consul AclToken
Posted by GitBox <gi...@apache.org>.
web-xiaxia commented on issue #4161: add Dynamic Configuration Consul AclToken
URL: https://github.com/apache/skywalking/pull/4161#issuecomment-570179294
> @web-xiaxia could you please also update the documentation? Codes look good to me, thanks
changed
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] web-xiaxia commented on a change in pull request
#4161: add Dynamic Configuration Consul AclToken
Posted by GitBox <gi...@apache.org>.
web-xiaxia commented on a change in pull request #4161: add Dynamic Configuration Consul AclToken
URL: https://github.com/apache/skywalking/pull/4161#discussion_r362404534
##########
File path: oap-server/server-bootstrap/src/main/resources/application.yml
##########
@@ -223,6 +225,8 @@ configuration:
# hostAndPorts: ${consul.address}
# # Sync period in seconds. Defaults to 60 seconds.
# period: 1
+# # Consul aclToken
+# #aclToken: ${consul.aclToken}
Review comment:
是不用改这个文件么？
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io edited a comment on issue #4161: add
Dynamic Configuration Consul AclToken
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4161: add Dynamic Configuration Consul AclToken
URL: https://github.com/apache/skywalking/pull/4161#issuecomment-570151024
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=h1) Report
> Merging [#4161](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/568c2e53f09855199884f65f38a9e4771a5a6467?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `0%`.
[](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4161 +/- ##
==========================================
- Coverage 26.89% 26.86% -0.03%
==========================================
Files 1160 1160
Lines 25377 25381 +4
Branches 3680 3682 +2
==========================================
- Hits 6824 6818 -6
- Misses 17945 17955 +10
Partials 608 608
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ter/plugin/consul/ClusterModuleConsulProvider.java](https://codecov.io/gh/apache/skywalking/pull/4161/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY2x1c3Rlci1wbHVnaW4vY2x1c3Rlci1jb25zdWwtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY2x1c3Rlci9wbHVnaW4vY29uc3VsL0NsdXN0ZXJNb2R1bGVDb25zdWxQcm92aWRlci5qYXZh) | `92.85% <0%> (-7.15%)` | :arrow_down: |
| [...ion/consul/ConsulConfigurationWatcherRegister.java](https://codecov.io/gh/apache/skywalking/pull/4161/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29uZmlndXJhdGlvbi9jb25maWd1cmF0aW9uLWNvbnN1bC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvbmZpZ3VyYXRpb24vY29uc3VsL0NvbnN1bENvbmZpZ3VyYXRpb25XYXRjaGVyUmVnaXN0ZXIuamF2YQ==) | `51.92% <0%> (-2.08%)` | :arrow_down: |
| [...apm/agent/core/remote/GRPCStreamServiceStatus.java](https://codecov.io/gh/apache/skywalking/pull/4161/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENTdHJlYW1TZXJ2aWNlU3RhdHVzLmphdmE=) | `29.16% <0%> (-25.01%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4161?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/skywalking/pull/4161?src=pr&el=footer). Last update [568c2e5...84c971e](https://codecov.io/gh/apache/skywalking/pull/4161?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
With regards,
Apache Git Services
[GitHub] [skywalking] web-xiaxia commented on a change in pull request
#4161: add Dynamic Configuration Consul AclToken
Posted by GitBox <gi...@apache.org>.
web-xiaxia commented on a change in pull request #4161: add Dynamic Configuration Consul AclToken
URL: https://github.com/apache/skywalking/pull/4161#discussion_r362436506
##########
File path: oap-server/server-cluster-plugin/cluster-consul-plugin/src/main/java/org/apache/skywalking/oap/server/cluster/plugin/consul/ClusterModuleConsulProvider.java
##########
@@ -76,6 +76,10 @@ public ClusterModuleConsulProvider() {
// we should set this value or it will be blocked forever
.withConnectTimeoutMillis(3000);
+ if (config.getAclToken() != null && !config.getAclToken().isEmpty()) {
Review comment:
Replacement completed
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io edited a comment on issue #4161: add
Dynamic Configuration Consul AclToken
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4161: add Dynamic Configuration Consul AclToken
URL: https://github.com/apache/skywalking/pull/4161#issuecomment-570151024
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=h1) Report
> Merging [#4161](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/568c2e53f09855199884f65f38a9e4771a5a6467?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `0%`.
[](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4161 +/- ##
==========================================
- Coverage 26.89% 26.86% -0.03%
==========================================
Files 1160 1160
Lines 25377 25381 +4
Branches 3680 3682 +2
==========================================
- Hits 6824 6818 -6
- Misses 17945 17955 +10
Partials 608 608
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ion/consul/ConsulConfigurationWatcherRegister.java](https://codecov.io/gh/apache/skywalking/pull/4161/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29uZmlndXJhdGlvbi9jb25maWd1cmF0aW9uLWNvbnN1bC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvbmZpZ3VyYXRpb24vY29uc3VsL0NvbnN1bENvbmZpZ3VyYXRpb25XYXRjaGVyUmVnaXN0ZXIuamF2YQ==) | `51.92% <0%> (-2.08%)` | :arrow_down: |
| [...apm/agent/core/remote/GRPCStreamServiceStatus.java](https://codecov.io/gh/apache/skywalking/pull/4161/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENTdHJlYW1TZXJ2aWNlU3RhdHVzLmphdmE=) | `29.16% <0%> (-25.01%)` | :arrow_down: |
| [...ter/plugin/consul/ClusterModuleConsulProvider.java](https://codecov.io/gh/apache/skywalking/pull/4161/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY2x1c3Rlci1wbHVnaW4vY2x1c3Rlci1jb25zdWwtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY2x1c3Rlci9wbHVnaW4vY29uc3VsL0NsdXN0ZXJNb2R1bGVDb25zdWxQcm92aWRlci5qYXZh) | `92.85% <0%> (-7.15%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4161?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/skywalking/pull/4161?src=pr&el=footer). Last update [568c2e5...aff505b](https://codecov.io/gh/apache/skywalking/pull/4161?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
With regards,
Apache Git Services
[GitHub] [skywalking] web-xiaxia commented on a change in pull request
#4161: add Dynamic Configuration Consul AclToken
Posted by GitBox <gi...@apache.org>.
web-xiaxia commented on a change in pull request #4161: add Dynamic Configuration Consul AclToken
URL: https://github.com/apache/skywalking/pull/4161#discussion_r362408626
##########
File path: oap-server/server-bootstrap/src/main/resources/application.yml
##########
@@ -223,6 +225,8 @@ configuration:
# hostAndPorts: ${consul.address}
# # Sync period in seconds. Defaults to 60 seconds.
# period: 1
+# # Consul aclToken
+# #aclToken: ${consul.aclToken}
Review comment:
configuration model and cluster model have changed
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io commented on issue #4161: add Dynamic
Configuration Consul AclToken
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #4161: add Dynamic Configuration Consul AclToken
URL: https://github.com/apache/skywalking/pull/4161#issuecomment-570151024
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=h1) Report
> Merging [#4161](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/568c2e53f09855199884f65f38a9e4771a5a6467?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `0%`.
[](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4161 +/- ##
==========================================
- Coverage 26.89% 26.86% -0.03%
==========================================
Files 1160 1160
Lines 25377 25379 +2
Branches 3680 3681 +1
==========================================
- Hits 6824 6818 -6
- Misses 17945 17954 +9
+ Partials 608 607 -1
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ion/consul/ConsulConfigurationWatcherRegister.java](https://codecov.io/gh/apache/skywalking/pull/4161/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29uZmlndXJhdGlvbi9jb25maWd1cmF0aW9uLWNvbnN1bC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvbmZpZ3VyYXRpb24vY29uc3VsL0NvbnN1bENvbmZpZ3VyYXRpb25XYXRjaGVyUmVnaXN0ZXIuamF2YQ==) | `51.92% <0%> (-2.08%)` | :arrow_down: |
| [...apm/agent/core/remote/GRPCStreamServiceStatus.java](https://codecov.io/gh/apache/skywalking/pull/4161/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENTdHJlYW1TZXJ2aWNlU3RhdHVzLmphdmE=) | `29.16% <0%> (-25.01%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4161?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/skywalking/pull/4161?src=pr&el=footer). Last update [568c2e5...aff505b](https://codecov.io/gh/apache/skywalking/pull/4161?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
With regards,
Apache Git Services
[GitHub] [skywalking] wu-sheng commented on a change in pull request #4161:
add Dynamic Configuration Consul AclToken
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #4161: add Dynamic Configuration Consul AclToken
URL: https://github.com/apache/skywalking/pull/4161#discussion_r362408035
##########
File path: oap-server/server-bootstrap/src/main/resources/application.yml
##########
@@ -223,6 +225,8 @@ configuration:
# hostAndPorts: ${consul.address}
# # Sync period in seconds. Defaults to 60 seconds.
# period: 1
+# # Consul aclToken
+# #aclToken: ${consul.aclToken}
Review comment:
English, please. You are changing two places, one is cluster model, the other is configuration model. Both of them have Consul implementations, but they are totally different.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [skywalking] codecov-io edited a comment on issue #4161: add
Dynamic Configuration Consul AclToken
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #4161: add Dynamic Configuration Consul AclToken
URL: https://github.com/apache/skywalking/pull/4161#issuecomment-570151024
# [Codecov](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=h1) Report
> Merging [#4161](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=desc) into [master](https://codecov.io/gh/apache/skywalking/commit/568c2e53f09855199884f65f38a9e4771a5a6467?src=pr&el=desc) will **decrease** coverage by `0.07%`.
> The diff coverage is `0%`.
[](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #4161 +/- ##
==========================================
- Coverage 26.89% 26.81% -0.08%
==========================================
Files 1160 1160
Lines 25377 25381 +4
Branches 3680 3682 +2
==========================================
- Hits 6824 6806 -18
- Misses 17945 17970 +25
+ Partials 608 605 -3
```
| [Impacted Files](https://codecov.io/gh/apache/skywalking/pull/4161?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...ter/plugin/consul/ClusterModuleConsulProvider.java](https://codecov.io/gh/apache/skywalking/pull/4161/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY2x1c3Rlci1wbHVnaW4vY2x1c3Rlci1jb25zdWwtcGx1Z2luL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9za3l3YWxraW5nL29hcC9zZXJ2ZXIvY2x1c3Rlci9wbHVnaW4vY29uc3VsL0NsdXN0ZXJNb2R1bGVDb25zdWxQcm92aWRlci5qYXZh) | `92.85% <0%> (-7.15%)` | :arrow_down: |
| [...ion/consul/ConsulConfigurationWatcherRegister.java](https://codecov.io/gh/apache/skywalking/pull/4161/diff?src=pr&el=tree#diff-b2FwLXNlcnZlci9zZXJ2ZXItY29uZmlndXJhdGlvbi9jb25maWd1cmF0aW9uLWNvbnN1bC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2t5d2Fsa2luZy9vYXAvc2VydmVyL2NvbmZpZ3VyYXRpb24vY29uc3VsL0NvbnN1bENvbmZpZ3VyYXRpb25XYXRjaGVyUmVnaXN0ZXIuamF2YQ==) | `51.92% <0%> (-2.08%)` | :arrow_down: |
| [...apm/agent/core/remote/GRPCStreamServiceStatus.java](https://codecov.io/gh/apache/skywalking/pull/4161/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL0dSUENTdHJlYW1TZXJ2aWNlU3RhdHVzLmphdmE=) | `29.16% <0%> (-25.01%)` | :arrow_down: |
| [.../core/remote/ServiceAndEndpointRegisterClient.java](https://codecov.io/gh/apache/skywalking/pull/4161/diff?src=pr&el=tree#diff-YXBtLXNuaWZmZXIvYXBtLWFnZW50LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NreXdhbGtpbmcvYXBtL2FnZW50L2NvcmUvcmVtb3RlL1NlcnZpY2VBbmRFbmRwb2ludFJlZ2lzdGVyQ2xpZW50LmphdmE=) | `31.46% <0%> (-13.49%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/skywalking/pull/4161?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/skywalking/pull/4161?src=pr&el=footer). Last update [568c2e5...84c971e](https://codecov.io/gh/apache/skywalking/pull/4161?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
With regards,
Apache Git Services