You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by liuyuzhong7 <gi...@git.apache.org> on 2017/01/17 11:59:03 UTC
[GitHub] flink pull request #3138: #Flink-5522 Storm Local Cluster can't work with po...
GitHub user liuyuzhong7 opened a pull request:
https://github.com/apache/flink/pull/3138
#Flink-5522 Storm Local Cluster can't work with powermock
Storm Local Cluster can't work with powermock.
Delete it because WrapperSetupHelperTest.testCreateTopologyContext doesn't need LocalCluster,
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/liuyuzhong7/flink FLINK-5522
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/3138.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #3138
----
commit 02596f71f8c1ac77b0ade49113a80b07f15cd337
Author: yuzhongliu <yu...@tencent.com>
Date: 2016-12-22T03:36:37Z
#FLINK-4450 update storm version to 1.0
commit 1dfb469bd6e8d16b18c554c52486477c48dcd3c8
Author: yuzhongliu <yu...@tencent.com>
Date: 2017-01-17T02:41:59Z
# fix confilct
commit 173913f669c1776572c97384fd4a44409bbecc33
Author: yuzhongliu <yu...@tencent.com>
Date: 2017-01-17T08:37:04Z
#4450 keep same code with apache/flink
commit 9925bf6f7301b50878e1a0a660ec9e41a7a294ce
Author: liuyuzhong7 <li...@gmail.com>
Date: 2017-01-17T08:40:25Z
Merge pull request #1 from liuyuzhong7/4450-1
#4450 keep same code with apache/flink
commit e51c705290e4394c957480914d9420bcb8cb7657
Author: liuyuzhong7 <li...@gmail.com>
Date: 2017-01-17T11:10:25Z
Merge remote-tracking branch 'upstream/master'
commit b9bac45ebe59bd805c5ae73b09aff057959df131
Author: liuyuzhong7 <li...@gmail.com>
Date: 2017-01-17T11:55:42Z
#FLINK-5522 delete Local Cluster test because it can't work with powermock
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3138: #Flink-5522 Storm Local Cluster can't work with powermock
Posted by liuyuzhong7 <gi...@git.apache.org>.
Github user liuyuzhong7 commented on the issue:
https://github.com/apache/flink/pull/3138
@zentol Right, Strom LocalCluster can run in a single class without powermock.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3138: #Flink-5522 Storm Local Cluster can't work with powermock
Posted by liuyuzhong7 <gi...@git.apache.org>.
Github user liuyuzhong7 commented on the issue:
https://github.com/apache/flink/pull/3138
@StephanEwen What should to to with this pull request?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3138: #Flink-5522 Storm Local Cluster can't work with powermock
Posted by liuyuzhong7 <gi...@git.apache.org>.
Github user liuyuzhong7 commented on the issue:
https://github.com/apache/flink/pull/3138
@zentol @StephanEwen Please help me to review this PR. 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3138: #Flink-5522 Storm Local Cluster can't work with powermock
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3138
I think that is a good fix, thank you!
Merging this...
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3138: #Flink-5522 Storm Local Cluster can't work with powermock
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/3138
I played around a bit with ignore pattern but couldn't get it to work.
However, moving WrapperSetupHelperTest into a separate class and replacing the StreamingRuntimeContext mocking (which in turn removes the need for PowerMock) should do the trick.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3138: #Flink-5522 Storm Local Cluster can't work with powermock
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/3138
The `org.apache.storm.LocalCluster` import can be removed as well.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3138: #Flink-5522 Storm Local Cluster can't work with powermock
Posted by liuyuzhong7 <gi...@git.apache.org>.
Github user liuyuzhong7 commented on the issue:
https://github.com/apache/flink/pull/3138
OK
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3138: #Flink-5522 Storm Local Cluster can't work with powermock
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3138
@liuyuzhong7 Would be good to know if you plan to follow up on this issue.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3138: #Flink-5522 Storm Local Cluster can't work with powermock
Posted by liuyuzhong7 <gi...@git.apache.org>.
Github user liuyuzhong7 commented on the issue:
https://github.com/apache/flink/pull/3138
@StephanEwen
Only LocalCluster in storm can't workwith powermock.
And it fail to init if ignore LocalCluster by powermock in WrapperSetupHelperTest.
So I think delete or give a new example for LocalCluster is the better solution.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request #3138: #Flink-5522 Storm Local Cluster can't work with po...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/3138
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3138: #Flink-5522 Storm Local Cluster can't work with powermock
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/3138
merging, will fix the import while doing so.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3138: #Flink-5522 Storm Local Cluster can't work with powermock
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3138
I think the correct fix would be not to remove the code, but
- either make sure that storm classes are ignored by powermnock (add an ignore pattern)
- rework the test such that powermock is not needed
How about that?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3138: #Flink-5522 Storm Local Cluster can't work with powermock
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/3138
It would be good to add the removed code back in a way that does not conflict with powermock, like @zentol suggested.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink issue #3138: #Flink-5522 Storm Local Cluster can't work with powermock
Posted by liuyuzhong7 <gi...@git.apache.org>.
Github user liuyuzhong7 commented on the issue:
https://github.com/apache/flink/pull/3138
@StephanEwen Fixed
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---