[STORM-2602] storm.zookeeper.topology.auth.payload doesn't work even you set it #2180
Conversation
|
@HeartSaVioR |
|
Sorry I don't know the detail of this part and not sure this is a bug. Have you faced specific issue regarding this bug? |
|
@HeartSaVioR |
|
I read through the code and I would like to confirm how you are setting the payload. It looks like you are setting the payload in the storm.yaml file, and not passing it in on the command line, or setting it programmatically when submitting your topology. Is that correct? If so then that is a use case I didn't anticipate when I first wrote the code and this change is fine, except for some indentation issues. |
|
On a related note this change looks to have broken So please take a look at why it broke. It probably just needs to be updated to always expect a payload to be returned. |
|
|
||
| String secretPayload = generateZookeeperDigestSecretPayload(); | ||
| secretPayload = generateZookeeperDigestSecretPayload(); | ||
| } | ||
| toRet.put(Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD, secretPayload); | ||
| LOG.info("Generated ZooKeeper secret payload for MD5-digest: " + secretPayload); |
There was a problem hiding this comment.
Please move this line up to be right below line 87. We don't want to log that we generated a payload unless we actually did.
|
|
||
| String secretPayload = generateZookeeperDigestSecretPayload(); | ||
| secretPayload = generateZookeeperDigestSecretPayload(); | ||
| } | ||
| toRet.put(Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD, secretPayload); |
There was a problem hiding this comment.
Please adjust the indentation here.
|
@revans2 |
|
@HeartSaVioR |
|
@HeartSaVioR |
|
@hmcl |
|
@HeartSaVioR |
|
@HeartSaVioR |
|
@HeartSaVioR |
|
@vesense |
|
@revans2 |
|
@srdo |
|
@harshach |
|
+1 |
|
@HeartSaVioR |
|
Could anyone help me merge this PR?It has been create long long ago.I only want to handle it properly.Could anyone reply to me? |
|
+1 Btw, unfortunately we're busy with handling our own works as well, whether it is related to Storm project or not. I understand the pain when review process is dragging as I was a one of contributor for more than a year, but please take into account the fact that the review process for PR could be dragging even for months, which I really would like to avoid, but we can't keep up every time. You may want to take a look at other pull requests in Storm project, or other OSPs to see how others leave comments for kindly reminder. |
|
@HeartSaVioR |
https://issues.apache.org/jira/browse/STORM-2602
"storm.zookeeper.topology.auth.payload" doesn't work even you have set it,because there doesn't use it when the value of STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD isn't null or other abnormal condition.