Yikun edited a comment on pull request #35820:
URL: https://github.com/apache/spark/pull/35820#issuecomment-1065838723


   > `deleteYAMLResource` is also async and we don't do double-check.
   
   `deleteYAMLResource` is [sync 
operation](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/VolcanoTestsSuite.scala#L66-L70),
 I think it is need, to make sure keep the independence of each test. Just like 
[other test resource 
cleanup](https://github.com/apache/spark/blob/c91c2e9afec0d5d5bbbd2e155057fe409c5bb928/resource-managers/kubernetes/integration-tests/src/test/scala/org/apache/spark/deploy/k8s/integrationtest/KubernetesSuite.scala#L585)
 in K8S IT.
   
   > In addition, what is the behavior of Volcano, for the pod group submission 
with non-existing queue name?
   
   Just like native sheduler, the driver will be pending status.
   
   > If Volcano hangs, we don't need this patch at all.
   
   Yes, without this patch, all tests are also passed as before. This patch is 
not only for `queue` (such as we also create priorities resource), It's just 
for more protective in my view. so, I will +1 for your choice,  merge or not 
merge is both fine for me.


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