You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "mxm (via GitHub)" <gi...@apache.org> on 2023/04/24 15:07:44 UTC

[GitHub] [flink-kubernetes-operator] mxm opened a new pull request, #574: [FLINK-31827] Discard old format scaling history

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

   We recently change the format for the metric history but this also affects the scaling history because it contains new / old scaling metrics.
   


-- 
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 commented on a diff in pull request #574: [FLINK-31827] Discard old format scaling history

Posted by "gyfora (via GitHub)" <gi...@apache.org>.
gyfora commented on code in PR #574:
URL: https://github.com/apache/flink-kubernetes-operator/pull/574#discussion_r1175451671


##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/AutoScalerInfo.java:
##########
@@ -95,7 +95,7 @@ public SortedMap<Instant, CollectedMetrics> getMetricHistory() {
 
         try {
             return YAML_MAPPER.readValue(decompress(historyYaml), new TypeReference<>() {});
-        } catch (JsonProcessingException e) {
+        } catch (JacksonException e) {

Review Comment:
   We could catch all `Exception`s maybe?



-- 
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] mxm commented on a diff in pull request #574: [FLINK-31827] Discard old format scaling history

Posted by "mxm (via GitHub)" <gi...@apache.org>.
mxm commented on code in PR #574:
URL: https://github.com/apache/flink-kubernetes-operator/pull/574#discussion_r1175535065


##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/AutoScalerInfo.java:
##########
@@ -95,7 +95,7 @@ public SortedMap<Instant, CollectedMetrics> getMetricHistory() {
 
         try {
             return YAML_MAPPER.readValue(decompress(historyYaml), new TypeReference<>() {});
-        } catch (JsonProcessingException e) {
+        } catch (JacksonException e) {

Review Comment:
   I deliberately did not catch Exception, just to avoid masking other errors, but we certainly can.



-- 
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] mxm commented on a diff in pull request #574: [FLINK-31827] Discard old format scaling history

Posted by "mxm (via GitHub)" <gi...@apache.org>.
mxm commented on code in PR #574:
URL: https://github.com/apache/flink-kubernetes-operator/pull/574#discussion_r1175710518


##########
flink-kubernetes-operator-autoscaler/src/main/java/org/apache/flink/kubernetes/operator/autoscaler/AutoScalerInfo.java:
##########
@@ -95,7 +95,7 @@ public SortedMap<Instant, CollectedMetrics> getMetricHistory() {
 
         try {
             return YAML_MAPPER.readValue(decompress(historyYaml), new TypeReference<>() {});
-        } catch (JsonProcessingException e) {
+        } catch (JacksonException e) {

Review Comment:
   The reason I changed the exception type is that this is the base Jackson exception which catches all Jackson related errors which we can't do anything about. Leaving this as-is for now.



-- 
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] mxm merged pull request #574: [FLINK-31827] Discard old format scaling history

Posted by "mxm (via GitHub)" <gi...@apache.org>.
mxm merged PR #574:
URL: https://github.com/apache/flink-kubernetes-operator/pull/574


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