You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/06/27 01:23:15 UTC

[GitHub] [flink-kubernetes-operator] tweise opened a new pull request, #281: [FLINK-26891] Record important deployment events

tweise opened a new pull request, #281:
URL: https://github.com/apache/flink-kubernetes-operator/pull/281

   Record events that may be of interest to users when looking at the deployment with describe. Generally, these will be conversions of existing log messages. I added couple examples for illustration, will add more like submit or trigger/complete savepoint if there is agreement on the approach.
   
   <img width="731" alt="image" src="https://user-images.githubusercontent.com/263695/175843449-cf08050c-3edd-4587-897a-97750d557a39.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.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] gyfora merged pull request #281: [FLINK-26891] Record important deployment events

Posted by GitBox <gi...@apache.org>.
gyfora merged PR #281:
URL: https://github.com/apache/flink-kubernetes-operator/pull/281


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] tweise commented on pull request #281: [FLINK-26891] Record important deployment events

Posted by GitBox <gi...@apache.org>.
tweise commented on PR #281:
URL: https://github.com/apache/flink-kubernetes-operator/pull/281#issuecomment-1168120050

   > Nice job, @tweise! I was wondering if we could/should make the Reason field part of the FlinkResourceListener API? If we do so a call control layer could leverage it and could build some business logic on top of it. Even if we don't make it part of the API yet an enum type with the possible reason values would enforce some naming convention and consistency. What do you think?
   
   Great suggestion. We could make the reason codes an enum similar to component and type. How about we then also change the signature of `triggerEvent` to use the enum instead of string?


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] morhidi commented on pull request #281: [FLINK-26891] Record important deployment events

Posted by GitBox <gi...@apache.org>.
morhidi commented on PR #281:
URL: https://github.com/apache/flink-kubernetes-operator/pull/281#issuecomment-1168242595

   > > Nice job, @tweise! I was wondering if we could/should make the Reason field part of the FlinkResourceListener API? If we do so a call control layer could leverage it and could build some business logic on top of it. Even if we don't make it part of the API yet an enum type with the possible reason values would enforce some naming convention and consistency. What do you think?
   > 
   > Great suggestion. We could make the reason codes an enum similar to component and type. How about we then also change the signature of `triggerEvent` to use the enum instead of string?
   
   Sounds good!


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] morhidi commented on pull request #281: [FLINK-26891] Record important deployment events

Posted by GitBox <gi...@apache.org>.
morhidi commented on PR #281:
URL: https://github.com/apache/flink-kubernetes-operator/pull/281#issuecomment-1167073080

   Nice job, @tweise! I was wondering if we could/should make the Reason field part of the FlinkResourceListener API? If we do so a call control layer could leverage it and could build some business logic on top of it. Even if we don't make it part of the API yet an enum type with the possible reason values would enforce some naming convention and consistency. What do you think?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [flink-kubernetes-operator] tweise commented on pull request #281: [FLINK-26891] Record important deployment events

Posted by GitBox <gi...@apache.org>.
tweise commented on PR #281:
URL: https://github.com/apache/flink-kubernetes-operator/pull/281#issuecomment-1169419765

   > The PR looks pretty good!
   > 
   > My only question whether we should trigger events before or after executing the actions using the Flink service.
   > 
   > For example now we trigger a suspended event before trying to cancel the job. So it might happen that the event is there but the job is not suspended. Of course we could have the reverse situation if we changed the order. Just wanted to discuss this explicitly.
   > 
   > What do you think @tweise ?
   
   It should be OK to have the event logged first. If the subsequent action fails, we have a trace that it was attempted and any subsequent error would yield anther event so that there should not be any confusion.
   
   I pulled the reason codes into `EventRecorder`. (I did not add them to the listener as the listener does not actually reference them and it would also be a bit unusual to have them there from the operator code caller perspective.)
   
   @gyfora @morhidi PTAL


-- 
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: issues-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org