You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "lyy-pineapple (via GitHub)" <gi...@apache.org> on 2024/01/12 06:44:21 UTC

[PR] [SPARK-46696][CORE] In ResourceProfileManager, function calls should occur after variable declarations [spark]

lyy-pineapple opened a new pull request, #44705:
URL: https://github.com/apache/spark/pull/44705

   ### What changes were proposed in this pull request?
   In ResourceProfileManager, function calls should occur after variable declarations
   
   
   ### Why are the changes needed?
   As the title suggests, in `ResourceProfileManager`, function calls should be made after variable declarations. When determining `isSupport`, all variables are uninitialized, with booleans defaulting to false and objects to null. While the end result is correct, the evaluation process is abnormal.
   ![image](https://github.com/apache/spark/assets/46274164/0e15b7e6-bd91-4d46-b220-758c131392c7)
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   
   
   ### How was this patch tested?
   through exists uts
   
   
   ### Was this patch authored or co-authored using generative AI tooling?
   No
   


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46696][CORE] In ResourceProfileManager, function calls should occur after variable declarations [spark]

Posted by "lyy-pineapple (via GitHub)" <gi...@apache.org>.
lyy-pineapple commented on PR #44705:
URL: https://github.com/apache/spark/pull/44705#issuecomment-1895237317

   @tgravescs @mridulm  thanks for review, do you have any further comments?


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46696][CORE] In ResourceProfileManager, function calls should occur after variable declarations [spark]

Posted by "lyy-pineapple (via GitHub)" <gi...@apache.org>.
lyy-pineapple commented on PR #44705:
URL: https://github.com/apache/spark/pull/44705#issuecomment-1891185867

   
   
   
   
   > Can you add a test to ensure this does not recur in future ? +CC @tgravescs
   
   
   
   > Can you add a test to ensure this does not recur in future ? +CC @tgravescs
   
   Adding tests can be challenging because all processes are within the initialization. To avoid this issue, we can add `assert(master != null)` in the function. Alternatively, consider changing the following variables to `lazy val `to ensure they are initialized before usage.I have chosen to include an assert statement here.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46696][CORE] In ResourceProfileManager, function calls should occur after variable declarations [spark]

Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm closed pull request #44705: [SPARK-46696][CORE] In ResourceProfileManager, function calls should occur after variable declarations
URL: https://github.com/apache/spark/pull/44705


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46696][CORE] In ResourceProfileManager, function calls should occur after variable declarations [spark]

Posted by "tgravescs (via GitHub)" <gi...@apache.org>.
tgravescs commented on PR #44705:
URL: https://github.com/apache/spark/pull/44705#issuecomment-1890087749

   change looks fine to me, thanks for fixing


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-46696][CORE] In ResourceProfileManager, function calls should occur after variable declarations [spark]

Posted by "mridulm (via GitHub)" <gi...@apache.org>.
mridulm commented on PR #44705:
URL: https://github.com/apache/spark/pull/44705#issuecomment-1898006973

   Merged to master.
   Thanks for fixing this @lyy-pineapple !
   Thanks for the review @tgravescs :-)


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org