You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by GitBox <gi...@apache.org> on 2021/06/03 18:45:18 UTC
[GitHub] [storm] agresch opened a new pull request #3399: STORM-3775 validate optional boolean fields in topology.blobstore.map
agresch opened a new pull request #3399:
URL: https://github.com/apache/storm/pull/3399
## What is the purpose of the change
Prevent invalid blobstore map conf settings from restarting supervisors.
## How was the change tested
Ran new unit tests. Validated submission of invalid boolean setting fails topology submission.
--
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:
users@infra.apache.org
[GitHub] [storm] agresch merged pull request #3399: STORM-3775 validate optional boolean fields in topology.blobstore.map
Posted by GitBox <gi...@apache.org>.
agresch merged pull request #3399:
URL: https://github.com/apache/storm/pull/3399
--
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:
users@infra.apache.org
[GitHub] [storm] Ethanlm commented on a change in pull request #3399: STORM-3775 validate optional boolean fields in topology.blobstore.map
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on a change in pull request #3399:
URL: https://github.com/apache/storm/pull/3399#discussion_r651902997
##########
File path: storm-client/src/jvm/org/apache/storm/utils/Utils.java
##########
@@ -1233,9 +1232,18 @@ public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf) th
*/
public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf, NimbusBlobStore client)
throws InvalidTopologyException, AuthorizationException {
- Map<String, Object> blobStoreMap = (Map<String, Object>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
+ Map<String, Map<String, Object>> blobStoreMap = (Map<String, Map<String, Object>>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
if (blobStoreMap != null) {
for (String key : blobStoreMap.keySet()) {
+
+ Map<String, Object> blobConf = blobStoreMap.get(key);
+ try {
+ ObjectReader.getBoolean(blobConf.get("uncompress"), false);
+ ObjectReader.getBoolean(blobConf.get("workerRestart"), false);
Review comment:
Maybe we should fix the comment here too
https://github.com/apache/storm/blob/master/storm-server/src/main/java/org/apache/storm/daemon/supervisor/SupervisorUtils.java#L122-L140
--
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:
users@infra.apache.org
[GitHub] [storm] Ethanlm commented on a change in pull request #3399: STORM-3775 validate optional boolean fields in topology.blobstore.map
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on a change in pull request #3399:
URL: https://github.com/apache/storm/pull/3399#discussion_r651902997
##########
File path: storm-client/src/jvm/org/apache/storm/utils/Utils.java
##########
@@ -1233,9 +1232,18 @@ public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf) th
*/
public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf, NimbusBlobStore client)
throws InvalidTopologyException, AuthorizationException {
- Map<String, Object> blobStoreMap = (Map<String, Object>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
+ Map<String, Map<String, Object>> blobStoreMap = (Map<String, Map<String, Object>>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
if (blobStoreMap != null) {
for (String key : blobStoreMap.keySet()) {
+
+ Map<String, Object> blobConf = blobStoreMap.get(key);
+ try {
+ ObjectReader.getBoolean(blobConf.get("uncompress"), false);
+ ObjectReader.getBoolean(blobConf.get("workerRestart"), false);
Review comment:
Where doss "workerRestart" come from?
--
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:
users@infra.apache.org
[GitHub] [storm] Ethanlm commented on a change in pull request #3399: STORM-3775 validate optional boolean fields in topology.blobstore.map
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on a change in pull request #3399:
URL: https://github.com/apache/storm/pull/3399#discussion_r651914437
##########
File path: storm-client/src/jvm/org/apache/storm/utils/Utils.java
##########
@@ -1233,9 +1232,18 @@ public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf) th
*/
public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf, NimbusBlobStore client)
throws InvalidTopologyException, AuthorizationException {
- Map<String, Object> blobStoreMap = (Map<String, Object>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
+ Map<String, Map<String, Object>> blobStoreMap = (Map<String, Map<String, Object>>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
if (blobStoreMap != null) {
for (String key : blobStoreMap.keySet()) {
+
+ Map<String, Object> blobConf = blobStoreMap.get(key);
+ try {
+ ObjectReader.getBoolean(blobConf.get("uncompress"), false);
+ ObjectReader.getBoolean(blobConf.get("workerRestart"), false);
Review comment:
Maybe we should fix the comment here too
https://github.com/apache/storm/blob/master/storm-server/src/main/java/org/apache/storm/daemon/supervisor/SupervisorUtils.java#L122-L140
--
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:
users@infra.apache.org
[GitHub] [storm] agresch commented on a change in pull request #3399: STORM-3775 validate optional boolean fields in topology.blobstore.map
Posted by GitBox <gi...@apache.org>.
agresch commented on a change in pull request #3399:
URL: https://github.com/apache/storm/pull/3399#discussion_r651908956
##########
File path: storm-client/src/jvm/org/apache/storm/utils/Utils.java
##########
@@ -1233,9 +1232,18 @@ public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf) th
*/
public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf, NimbusBlobStore client)
throws InvalidTopologyException, AuthorizationException {
- Map<String, Object> blobStoreMap = (Map<String, Object>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
+ Map<String, Map<String, Object>> blobStoreMap = (Map<String, Map<String, Object>>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
if (blobStoreMap != null) {
for (String key : blobStoreMap.keySet()) {
+
+ Map<String, Object> blobConf = blobStoreMap.get(key);
+ try {
+ ObjectReader.getBoolean(blobConf.get("uncompress"), false);
+ ObjectReader.getBoolean(blobConf.get("workerRestart"), false);
Review comment:
AsyncLocalizer calls this indirectly: https://github.com/apache/storm/blob/master/storm-server/src/main/java/org/apache/storm/daemon/supervisor/SupervisorUtils.java#L138
--
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:
users@infra.apache.org
[GitHub] [storm] agresch commented on a change in pull request #3399: STORM-3775 validate optional boolean fields in topology.blobstore.map
Posted by GitBox <gi...@apache.org>.
agresch commented on a change in pull request #3399:
URL: https://github.com/apache/storm/pull/3399#discussion_r651917143
##########
File path: storm-client/src/jvm/org/apache/storm/utils/Utils.java
##########
@@ -1233,9 +1232,18 @@ public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf) th
*/
public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf, NimbusBlobStore client)
throws InvalidTopologyException, AuthorizationException {
- Map<String, Object> blobStoreMap = (Map<String, Object>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
+ Map<String, Map<String, Object>> blobStoreMap = (Map<String, Map<String, Object>>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
if (blobStoreMap != null) {
for (String key : blobStoreMap.keySet()) {
+
+ Map<String, Object> blobConf = blobStoreMap.get(key);
+ try {
+ ObjectReader.getBoolean(blobConf.get("uncompress"), false);
+ ObjectReader.getBoolean(blobConf.get("workerRestart"), false);
Review comment:
I will update to indicate only a boolean is supported.
--
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:
users@infra.apache.org
[GitHub] [storm] agresch commented on a change in pull request #3399: STORM-3775 validate optional boolean fields in topology.blobstore.map
Posted by GitBox <gi...@apache.org>.
agresch commented on a change in pull request #3399:
URL: https://github.com/apache/storm/pull/3399#discussion_r651919634
##########
File path: storm-client/src/jvm/org/apache/storm/utils/Utils.java
##########
@@ -1233,9 +1232,18 @@ public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf) th
*/
public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf, NimbusBlobStore client)
throws InvalidTopologyException, AuthorizationException {
- Map<String, Object> blobStoreMap = (Map<String, Object>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
+ Map<String, Map<String, Object>> blobStoreMap = (Map<String, Map<String, Object>>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
if (blobStoreMap != null) {
for (String key : blobStoreMap.keySet()) {
+
+ Map<String, Object> blobConf = blobStoreMap.get(key);
+ try {
+ ObjectReader.getBoolean(blobConf.get("uncompress"), false);
+ ObjectReader.getBoolean(blobConf.get("workerRestart"), false);
Review comment:
updated
--
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:
users@infra.apache.org
[GitHub] [storm] Ethanlm commented on a change in pull request #3399: STORM-3775 validate optional boolean fields in topology.blobstore.map
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on a change in pull request #3399:
URL: https://github.com/apache/storm/pull/3399#discussion_r651902997
##########
File path: storm-client/src/jvm/org/apache/storm/utils/Utils.java
##########
@@ -1233,9 +1232,18 @@ public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf) th
*/
public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf, NimbusBlobStore client)
throws InvalidTopologyException, AuthorizationException {
- Map<String, Object> blobStoreMap = (Map<String, Object>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
+ Map<String, Map<String, Object>> blobStoreMap = (Map<String, Map<String, Object>>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
if (blobStoreMap != null) {
for (String key : blobStoreMap.keySet()) {
+
+ Map<String, Object> blobConf = blobStoreMap.get(key);
+ try {
+ ObjectReader.getBoolean(blobConf.get("uncompress"), false);
+ ObjectReader.getBoolean(blobConf.get("workerRestart"), false);
Review comment:
Where does "workerRestart" come from?
--
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:
users@infra.apache.org
[GitHub] [storm] Ethanlm commented on a change in pull request #3399: STORM-3775 validate optional boolean fields in topology.blobstore.map
Posted by GitBox <gi...@apache.org>.
Ethanlm commented on a change in pull request #3399:
URL: https://github.com/apache/storm/pull/3399#discussion_r651916303
##########
File path: storm-client/src/jvm/org/apache/storm/utils/Utils.java
##########
@@ -1233,9 +1232,18 @@ public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf) th
*/
public static void validateTopologyBlobStoreMap(Map<String, Object> topoConf, NimbusBlobStore client)
throws InvalidTopologyException, AuthorizationException {
- Map<String, Object> blobStoreMap = (Map<String, Object>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
+ Map<String, Map<String, Object>> blobStoreMap = (Map<String, Map<String, Object>>) topoConf.get(Config.TOPOLOGY_BLOBSTORE_MAP);
if (blobStoreMap != null) {
for (String key : blobStoreMap.keySet()) {
+
+ Map<String, Object> blobConf = blobStoreMap.get(key);
+ try {
+ ObjectReader.getBoolean(blobConf.get("uncompress"), false);
+ ObjectReader.getBoolean(blobConf.get("workerRestart"), false);
Review comment:
Maybe we should fix the comment here too
https://github.com/apache/storm/blob/master/storm-server/src/main/java/org/apache/storm/daemon/supervisor/SupervisorUtils.java#L122-L140
--
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:
users@infra.apache.org
[GitHub] [storm] agresch merged pull request #3399: STORM-3775 validate optional boolean fields in topology.blobstore.map
Posted by GitBox <gi...@apache.org>.
agresch merged pull request #3399:
URL: https://github.com/apache/storm/pull/3399
--
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:
users@infra.apache.org