tillrohrmann commented on a change in pull request #10852:
[FLINK-15564][yarn][test] Fix YarnClusterDescriptorTest that failed to validate
the original intended behavior
URL: https://github.com/apache/flink/pull/10852#discussion_r367350982
##########
File path:
flink-yarn/src/main/java/org/apache/flink/yarn/YarnClusterDescriptor.java
##########
@@ -318,6 +308,20 @@ private void isReadyForDeployment(ClusterSpecification
clusterSpecification) thr
}
}
+ @VisibleForTesting
+ protected int getNumYarnMaxVcores() throws YarnDeploymentException {
Review comment:
I agree with Andrey that overriding production code methods should always be
our last resort when it comes to testing. The danger is that this method
evolves and that we override important behaviour unintentionally. I would
propose two solutions to the problem:
1. Introduce a `YarnClusterInformationRetriever` interface which offers the
method `getMaxVcores`. The default implementation will simply use the
`YarnClient` to retrieve the max vcores. In the test we can provide a testing
implementation.
2. Alternatively, similar to `TestingYarnClient` override the
`getNodeReports` method from `YarnClientImpl`.
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
