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 2021/05/02 08:24:32 UTC

[GitHub] [skywalking-query-protocol] chenmudu opened a new pull request #57: include event to AlarmMessage.

chenmudu opened a new pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57


   


-- 
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] [skywalking-query-protocol] chenmudu commented on pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
chenmudu commented on pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#issuecomment-830784369


   > After merge, you need to update the submodule in your branch about query-receiver.
   
   Copy.


-- 
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] [skywalking-query-protocol] chenmudu commented on pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
chenmudu commented on pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#issuecomment-830779224


   > And with this change, you need another new issue to track UI update.
   
   What should I do, Create a PR in the main repository to track UI updates or do I need to keep an eye on related UI updates?


-- 
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] [skywalking-query-protocol] chenmudu edited a comment on pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
chenmudu edited a comment on pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#issuecomment-830779224


   > And with this change, you need another new issue to track UI update.
   
   What should I do, Create a issue in the main repository to track UI updates or do I need to keep an eye on related UI updates?


-- 
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] [skywalking-query-protocol] chenmudu commented on a change in pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
chenmudu commented on a change in pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#discussion_r624661543



##########
File path: alarm.graphqls
##########
@@ -37,6 +38,32 @@ input AlarmTag {
     value: String
 }
 
+type Event {

Review comment:
       OK, Thanks a lot. 




-- 
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] [skywalking-query-protocol] wu-sheng commented on pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#issuecomment-830778146


   And with this change, you need another new issue to track UI update.


-- 
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] [skywalking-query-protocol] chenmudu commented on pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
chenmudu commented on pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#issuecomment-830780100


   > > > And with this change, you need another new issue to track UI update.
   > > 
   > > 
   > > What should I do, Create a issue in the main repository to track UI updates or do I need to keep an eye on related UI updates? 😀
   > 
   > Submit another issue, talk about what you are going to change, UI should visualize this too.
   
   Copy that. I'll describe it clearly. Thanks.


-- 
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] [skywalking-query-protocol] wu-sheng commented on a change in pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#discussion_r624660632



##########
File path: alarm.graphqls
##########
@@ -37,6 +38,32 @@ input AlarmTag {
     value: String
 }
 
+type Event {

Review comment:
       Yes, you could use it directly. And more importantly, you should try the latest version in your OAP, and make sure it is suitable(compilable and runnable). There are a lot of verifications when you boot the OAP server.




-- 
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] [skywalking-query-protocol] wu-sheng commented on a change in pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#discussion_r624659141



##########
File path: alarm.graphqls
##########
@@ -37,6 +38,32 @@ input AlarmTag {
     value: String
 }
 
+type Event {

Review comment:
       Please share the object in `event.graphqls`. You don't need this duplicate one.




-- 
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] [skywalking-query-protocol] wu-sheng merged pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57


   


-- 
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] [skywalking-query-protocol] wu-sheng commented on pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#issuecomment-830782103


   After merge, you need to update the submodule in your branch about query-receiver.


-- 
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] [skywalking-query-protocol] wu-sheng commented on pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#issuecomment-830779749


   > > And with this change, you need another new issue to track UI update.
   > 
   > What should I do, Create a issue in the main repository to track UI updates or do I need to keep an eye on related UI updates? 😀
   
   Submit another issue, talk about what you are going to change, UI should visualize this 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:
users@infra.apache.org

[GitHub] [skywalking-query-protocol] chenmudu commented on a change in pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
chenmudu commented on a change in pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#discussion_r624660731



##########
File path: alarm.graphqls
##########
@@ -24,6 +24,7 @@ type AlarmMessage {
     scope: Scope
     id: ID!
     message: String!
+    events: [Event]!

Review comment:
       Ok. my earlier fears were misplaced.




-- 
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] [skywalking-query-protocol] wu-sheng commented on a change in pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#discussion_r624659141



##########
File path: alarm.graphqls
##########
@@ -37,6 +38,32 @@ input AlarmTag {
     value: String
 }
 
+type Event {

Review comment:
       Please share the type in `event.graphqls`. You don't need this duplicate one.




-- 
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] [skywalking-query-protocol] wu-sheng commented on a change in pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#discussion_r624659250



##########
File path: alarm.graphqls
##########
@@ -24,6 +24,7 @@ type AlarmMessage {
     scope: Scope
     id: ID!
     message: String!
+    events: [Event]!

Review comment:
       ```suggestion
       events: [Event!]!
   ```




-- 
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] [skywalking-query-protocol] chenmudu edited a comment on pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
chenmudu edited a comment on pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#issuecomment-830779224


   > And with this change, you need another new issue to track UI update.
   
   What should I do, Create a issue in the main repository to track UI updates or do I need to keep an eye on related UI updates? 😀


-- 
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] [skywalking-query-protocol] wu-sheng commented on a change in pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#discussion_r624662236



##########
File path: alarm.graphqls
##########
@@ -37,6 +38,7 @@ input AlarmTag {
     value: String
 }
 
+

Review comment:
       ```suggestion
   ```
   
   Be careful, don't change an unnecessary thing, due to not confusing others.




-- 
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] [skywalking-query-protocol] chenmudu commented on a change in pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
chenmudu commented on a change in pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#discussion_r624663562



##########
File path: alarm.graphqls
##########
@@ -37,6 +38,7 @@ input AlarmTag {
     value: String
 }
 
+

Review comment:
       Yeah, Thanks a lot. 




-- 
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] [skywalking-query-protocol] chenmudu commented on a change in pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
chenmudu commented on a change in pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#discussion_r624660422



##########
File path: alarm.graphqls
##########
@@ -37,6 +38,32 @@ input AlarmTag {
     value: String
 }
 
+type Event {

Review comment:
       Can I apply the `Event` of event.graphqls directly to alarm.graphqls?  Sorry, I'm not familiar with this query protocol.
   Like this? 
   ![image](https://user-images.githubusercontent.com/36784140/116808054-5b809000-ab69-11eb-993b-3c253e565bf9.png)
   




-- 
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] [skywalking-query-protocol] chenmudu commented on a change in pull request #57: Include event to AlarmMessage.

Posted by GitBox <gi...@apache.org>.
chenmudu commented on a change in pull request #57:
URL: https://github.com/apache/skywalking-query-protocol/pull/57#discussion_r624662543



##########
File path: alarm.graphqls
##########
@@ -37,6 +38,7 @@ input AlarmTag {
     value: String
 }
 
+

Review comment:
       > Be careful, don't change an unnecessary thing, due to not confusing others.
   
   Yeah, I'm so sorry. I'll keep it in mind.




-- 
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