[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-660736322 @EvanLjp Still need you to polish this PR. 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-660097453 Let's make that decision first, then back to this 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-660097268 By thinking the PR and Kafka transport PR, I prefer you work on a new PR. Introduce a new SPI service, called ConfigInitService. All plugins and core could provide their services. Then you could separate the current Config in the core and different plugins. Notice, all config keys should be kept as much as possible. What do you think? @dmsolr @kezhenxu94 @arugal @EvanLjp 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-658031498 @innerpeacez Please recheck. 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-657240474 It is required, such as local mini kube. We can't afford very pr repeatedly by committer. They are busy too. 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-657237166 Please provide your local test logs to prove you have tested the codes. Otherwise, I would not recommend our committer to spend time on reviewing this PR. Changing the status to TBD. 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-657236965 And, you don't need `org.apache.skywalking.oap.server.library.module.ModuleProvider` duplicated in the test folder. They should share the same one in the `src`. 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-657236863  I am pretty sure you have not tested the codes locally. Please don't do this again. This folder is actually wrong, `META-INF.services` SHOULD be two folders, you are putting it as one and dot in the name. Again, TEST locally. 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-657235551 I and @innerpeacez are still trying to bootstrap the codes. The OAP even can't start up by your pull request. Please recheck locally, and save our time. 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-657230730 > yes , this code has been already running on our production env That is why I asked you make sure, the open source version should be tested again. 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-657223329 > i would like to fix this little bugs now , and i don't know how to add the e2e process . Recently, i am so busy on my work . prefer to add the e2e process in the next pr. is there some demo on how to add e2e process I haven't asked for that now, but when you have time, let's work on that later. My point is, your branch should be tested again locally after moving from your private fork to guarantee the release stability. 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-657179933 I guess these codes maybe run in your private env, because every time, you want to push the changes in the upstream, you should and need to test again on the new codebase. Otherwise, reviewer, such as @innerpeacez, has to cost a lot of time to check what is going on. 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-656529033 @innerpeacez More importantly, once it is changed, the OAP should receive that notification, and behavior should be changed, such as sampling rate. Switching to 10% from 100%(default) should be an easy way to test. 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-656522857 @innerpeacez The thing you are asking, we could discuss on another issue :) 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-656522669 @innerpeacez He is doing dynamic configuration, not static config override. So basically, he wants to change this in the runtime. Based on this, the name should follow the document, https://github.com/apache/skywalking/blob/master/docs/en/setup/backend/dynamic-config.md. Those names are shared across different platforms. 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-655819719 @EvanLjp A lot of CI tasks fail, what happens? Could you fix them? 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-653268512 @EvanLjp Please update the document. 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-652206008 It says your codes coverage rate is lower than the whole project. 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-652118990 There are still some issues about the CI process. 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-651583521 @EvanLjp Could you make this PR ready? 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: us...@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on pull request #4959: Configmap configuration
wu-sheng commented on pull request #4959: URL: https://github.com/apache/skywalking/pull/4959#issuecomment-647475836 Please run rat check, is there ASF 2.0 header missing? 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: us...@infra.apache.org
