[GitHub] storm pull request: STORM-492: Fixed bug from tracked-wait / Added...
Github user harshach commented on the pull request: https://github.com/apache/storm/pull/279#issuecomment-61831975 @clockfly this patch doesn't work. running this mvn clean install -DSTORM_TEST_TIMEOUT_MS=3 (let [timeout (System/getProperty STORM_TEST_TIMEOUT_MS)] the above code in testing.clj won't pick up the -D variable because STORM_TEST_TIMEOUT_MS is not a pom variable. As per the above reviews we asked for a change to System/getenv where users can export STORM_TEST_TIMEOUT_MS=1 and the getenv will pick up but looks like this got merged in without those changes . Can you please revert this patch. @HeartSaVioR Please update the PR with @ptgoetz suggestions. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-492: Fixed bug from tracked-wait / Added...
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/279#issuecomment-61885540 @ptgoetz @harshach @clockfly I did confused about it at that time. Sorry about my mistake. ;( I'll update it and post new PR. Regarding revert plan, actually only last commit makes problem. There was a bug in document, not a code so it may not required to revert all things. So we can go with reverting last commit and correcting README.md. Maybe I can take care of it with new PR. Let me know your opinions. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-492: Fixed bug from tracked-wait / Added...
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/279#issuecomment-61893230 I re-read my comments, and I found I've mistaken with talking to @ptgoetz . Actually I prefer JVM property, not system environment. Maybe I was confusing about it. But it seems be a harder way to support it (regarding propagation of property across JVM), and we're already OK to go with system environment, so I'll update it with system environment. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-492: Fixed bug from tracked-wait / Added...
Github user curtisallen commented on the pull request: https://github.com/apache/storm/pull/279#issuecomment-59975061 Any chance this can make it into v0.9.3-rc2? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-492: Fixed bug from tracked-wait / Added...
Github user HeartSaVioR commented on a diff in the pull request: https://github.com/apache/storm/pull/279#discussion_r18803151 --- Diff: DEVELOPER.md --- @@ -229,6 +229,9 @@ The following commands must be run from the top-level directory. # Build the code and run the tests (requires nodejs, python and ruby installed) $ mvn clean install +# Build the code and run the tests, with specifying default test timeout (in millisecond) +$ mvn clean install -DSTORM_TEST_TIMEOUT_MS=1 --- End diff -- @harshach @ptgoetz Could you advice me which is preferred way in Storm Project? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-492: Fixed bug from tracked-wait / Added...
Github user harshach commented on the pull request: https://github.com/apache/storm/pull/279#issuecomment-57910263 @HeartSaVioR overall this looks good to me and works as expected. Please add a note to DEVELOPER.md under Build the code and run the tests on setting the test-timeout-ms. I am +1 after the above fixes. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] storm pull request: STORM-492: Fixed bug from tracked-wait / Added...
Github user HeartSaVioR commented on the pull request: https://github.com/apache/storm/pull/279#issuecomment-57912017 @harshach I've reflected your suggestion. Thanks for reviewing! --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
