tgravescs commented on a change in pull request #28972:
URL: https://github.com/apache/spark/pull/28972#discussion_r450859812
##########
File path: core/src/main/scala/org/apache/spark/resource/ResourceProfile.scala
##########
@@ -245,6 +245,7 @@ object ResourceProfile extends Logging {
// Executor resources
val CORES = "cores"
val MEMORY = "memory"
+ val OFFHEAP_MEM = "offHeap"
val OVERHEAD_MEM = "memoryOverhead"
val PYSPARK_MEM = "pyspark.memory"
Review comment:
Sure, in ResourceProfileSuite, lets create a test that has every type of
resource in allSupportedExecutorResources (including off heap) and then say 2
custom resources. Then just call getCustomExecutorResources to ensure that you
only get the 2 custom resources back. We may also have it check the size of
allSupportedExecutorResources == 5, that way if someone modifies
allSupportedExecutorResources but doesn't update the test it will fail.
Could you also add a big comment after "// Executor resources" that say
make sure to update allSupportedExecutorResources
----------------------------------------------------------------
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]
