You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "adoroszlai (via GitHub)" <gi...@apache.org> on 2024/05/29 10:11:36 UTC

[PR] HDDS-10934. Deduplicate TestOzoneRpcClient and TestOzoneRpcClientWithRatis [ozone]

adoroszlai opened a new pull request, #6747:
URL: https://github.com/apache/ozone/pull/6747

   ## What changes were proposed in this pull request?
   
   `TestOzoneRpcClient` and `TestOzoneRpcClientWithRatis` are subclasses of `TestOzoneRpcClientAbstract.`  They used to execute the same test cases with OM Ratis disabled (relying on default setting) and enabled (explicitly), respectively.  HDDS-4498 enabled OM Ratis by default, so now both classes test with OM Ratis enabled.
   
   This change disables OM Ratis for `TestOzoneRpcClient`, and fixes one test case that was failing with NPE in this setup.
   
   https://issues.apache.org/jira/browse/HDDS-10934
   
   ## How was this patch tested?
   
   CI:
   https://github.com/adoroszlai/ozone/actions/runs/9282222672/job/25540423674#step:5:2276


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-10934. Refactor TestOzoneRpcClient hierarchy [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #6747:
URL: https://github.com/apache/ozone/pull/6747


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-10934. Refactor TestOzoneRpcClient hierarchy [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6747:
URL: https://github.com/apache/ozone/pull/6747#issuecomment-2142911300

   Thanks @errose28 for the review.


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-10934. Refactor TestOzoneRpcClient hierarchy [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6747:
URL: https://github.com/apache/ozone/pull/6747#issuecomment-2142843029

   @errose28 patch is updated, please take another look


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-10934. Deduplicate TestOzoneRpcClient and TestOzoneRpcClientWithRatis [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6747:
URL: https://github.com/apache/ozone/pull/6747#issuecomment-2138834684

   Thanks @errose28 for the review.
   
   > * The tests run for both ratis and non-ratis cases (in `TestOzoneRpcClientAbstract`) and those run only for Ratis cases (in `TestOzoneRpcClientWithRatis`) seem kind of arbitrary.
   >   
   >   * Maybe we can either unify them so all tests run in both setups, or only a minimal amount of validation is done in the non-ratis setup.
   >   * Also tests that are unique to `TestOzoneRpcClientWithRatis` are not run in a secure setup.
   
   Agree, we should move them to the parent class.
   
   > * `TestSecureOzoneRpcClient` extends `TestOzoneRpcClient`, so now the secure tests are only running in a non-ratis OM setup.
   
   This is not the case, since `TestSecureOzoneRpcClient` creates its own cluster, `TestOzoneRpcClient#init` is not invoked due to "shadowing" (which is a bit obscure).  Otherwise it would be testing an unsecure setup or starting two clusters.  This should be improved, too.
   
   Will push a few more commits.


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org