smengcl edited a comment on pull request #1601: URL: https://github.com/apache/ozone/pull/1601#issuecomment-731960577
> @smengcl Thanks the patch. It looks good to me overall. But I think we need more strong integration test to cover the case: StateContext#reports will not contain any ContainerReport, NodeReport, and DatanodeReport. Thanks for the review @runzhiwang . In a previous revision I had a strict check in `StateContext#addReport`: https://github.com/apache/ozone/pull/1601/commits/22846f48ce34f3fcefa1de2d3bd97c4771c26742#diff-4e474a0cbb28091ed4fab8ee80878c3acf7e15711fba2a0d83bacd56ea8f5707L237-L240 which ensures only `CommandStatusReports` and `IncrementalContainerReport` would ever be added to `reports` queue. Later this is removed since @avijayanhwx suggested we don't really need that. The only two paths where reports would be queued are `addReports` and `StateContext#putBackReports`, where `HeartbeatEndpointTask#putBackReports` makes sure we only put back `CommandStatusReports` and `IncrementalContainerReport`. Would you elaborate on the integration test plan you have on your 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: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
