wypoon commented on PR #4395:
URL: https://github.com/apache/iceberg/pull/4395#issuecomment-1105479689

   @RussellSpitzer thank you for reviewing again!
   The changes to the descriptions were discussed in an earlier review comment, 
and my understanding was that reviewers were in favor of the changes. They 
affect the Spark UI. That said, if you prefer, I will remove them and put them 
in a separate PR.
   I have tested this manually. My take is this: In Spark, there are unit tests 
for testing consumption of custom metrics by the SparkAppListener. Here we are 
just plugging into the API. On the Iceberg side, how is the metric we are 
adding computed? It is simply scanTask.files().size() as you point out. Scan 
planning is presumably tested, so there is really nothing essentially new that 
needs to be tested. In #4588, on the other hand, there are extensive changes 
needed to compute the metric being added, so I test that computation.
   
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to