You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2021/09/10 21:44:16 UTC
[GitHub] [geode] jinmeiliao opened a new pull request #6860: GEODE-9521: add more multi-server test scenario
jinmeiliao opened a new pull request #6860:
URL: https://github.com/apache/geode/pull/6860
* remove the static methods in ExpirableSecurityManager
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jinmeiliao commented on a change in pull request #6860: GEODE-9521: add more multi-server test scenario
Posted by GitBox <gi...@apache.org>.
jinmeiliao commented on a change in pull request #6860:
URL: https://github.com/apache/geode/pull/6860#discussion_r707747689
##########
File path: geode-junit/src/main/java/org/apache/geode/security/ExpirableSecurityManager.java
##########
@@ -31,44 +32,44 @@
*
* make sure to call reset after each test to clean things up.
*/
-public class ExpirableSecurityManager extends SimpleSecurityManager {
+public class ExpirableSecurityManager extends SimpleSecurityManager implements Serializable {
Review comment:
No, this is not production code. `ExpirableSecurityManager` is a test security manager in `geode-junit` module. Geode product doesn't provide any implementation of `SecurityManager`
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6860: GEODE-9521: add more multi-server test scenario
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6860:
URL: https://github.com/apache/geode/pull/6860#discussion_r707745200
##########
File path: geode-junit/src/main/java/org/apache/geode/security/ExpirableSecurityManager.java
##########
@@ -31,44 +32,44 @@
*
* make sure to call reset after each test to clean things up.
*/
-public class ExpirableSecurityManager extends SimpleSecurityManager {
+public class ExpirableSecurityManager extends SimpleSecurityManager implements Serializable {
Review comment:
This is `Serializable` only because of lambda style VM invocations, right?
Since this is in the product code and doesn't really need to go on the wire, you should instead subclass `ExpirableSecurityManager` as an inner-class within the dunit test and then use that subclass in the tests.
```
private static class SerializableExpirableSecurityManager
extends ExpirableSecurityManager
implements Serializable {
...
}
```
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jinmeiliao commented on a change in pull request #6860: GEODE-9521: add more multi-server test scenario
Posted by GitBox <gi...@apache.org>.
jinmeiliao commented on a change in pull request #6860:
URL: https://github.com/apache/geode/pull/6860#discussion_r707747689
##########
File path: geode-junit/src/main/java/org/apache/geode/security/ExpirableSecurityManager.java
##########
@@ -31,44 +32,44 @@
*
* make sure to call reset after each test to clean things up.
*/
-public class ExpirableSecurityManager extends SimpleSecurityManager {
+public class ExpirableSecurityManager extends SimpleSecurityManager implements Serializable {
Review comment:
No, this is not production code. `ExpirableSecurityManager` is a test security manager in `geode-junit` module
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6860: GEODE-9521: add more multi-server test scenario
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6860:
URL: https://github.com/apache/geode/pull/6860#discussion_r707745200
##########
File path: geode-junit/src/main/java/org/apache/geode/security/ExpirableSecurityManager.java
##########
@@ -31,44 +32,44 @@
*
* make sure to call reset after each test to clean things up.
*/
-public class ExpirableSecurityManager extends SimpleSecurityManager {
+public class ExpirableSecurityManager extends SimpleSecurityManager implements Serializable {
Review comment:
This is `Serializable only` because of lambda style VM invocations, right?
Since this is in the product code and doesn't really need to go on the wire, you should instead subclass `ExpirableSecurityManager` as an inner-class within the dunit test and then use that subclass in the tests.
```
private static class SerializableExpirableSecurityManager
extends ExpirableSecurityManager
implements Serializable {
...
}
```
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] jinmeiliao merged pull request #6860: GEODE-9521: add more multi-server test scenario
Posted by GitBox <gi...@apache.org>.
jinmeiliao merged pull request #6860:
URL: https://github.com/apache/geode/pull/6860
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [geode] kirklund commented on a change in pull request #6860: GEODE-9521: add more multi-server test scenario
Posted by GitBox <gi...@apache.org>.
kirklund commented on a change in pull request #6860:
URL: https://github.com/apache/geode/pull/6860#discussion_r707774913
##########
File path: geode-junit/src/main/java/org/apache/geode/security/ExpirableSecurityManager.java
##########
@@ -31,44 +32,44 @@
*
* make sure to call reset after each test to clean things up.
*/
-public class ExpirableSecurityManager extends SimpleSecurityManager {
+public class ExpirableSecurityManager extends SimpleSecurityManager implements Serializable {
Review comment:
Oh yeah I see `geode-junit` now. Thanks!
--
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: notifications-unsubscribe@geode.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org