You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by m50d <gi...@git.apache.org> on 2014/11/07 16:36:38 UTC
[GitHub] thrift pull request: Pull TAsyncProcessor interface out of TBaseAs...
GitHub user m50d opened a pull request:
https://github.com/apache/thrift/pull/253
Pull TAsyncProcessor interface out of TBaseAsyncProcessor
As briefly mentioned on IRC (I'm lmm). This makes it possible to use e.g. TNonblockingServer with a custom async processor that doesn't extend TBaseAsyncProcessor.
(Use case: I have an implementation in scrooge that generates such async processors, for use in scala).
Async processors still need to "implement" TProcessor so that they can be configured as a processor (the implementation can be simply `return false` as it is in `TBaseAsyncProcessor`); this is somewhat awkward but a change to make that unnecessary would be much more intrusive.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/m50d/thrift master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/thrift/pull/253.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 #253
----
commit 9cde8b7721f5d326602b36bb9978bdd3d458da25
Author: Michael Donaghy <mi...@visualdna.com>
Date: 2014-11-07T10:19:26Z
Attempt to add TAsyncProcessor
commit 3241078784defe1280ac410b9bb3def2c787084f
Author: Michael Donaghy <mi...@visualdna.com>
Date: 2014-11-07T14:10:26Z
comment the requirement
----
---
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] thrift pull request: Pull TAsyncProcessor interface out of TBaseAs...
Posted by bsideup <gi...@git.apache.org>.
Github user bsideup commented on the pull request:
https://github.com/apache/thrift/pull/253#issuecomment-96174553
:+1:
---
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] thrift pull request: Pull TAsyncProcessor interface out of TBaseAs...
Posted by m50d <gi...@git.apache.org>.
Github user m50d commented on the pull request:
https://github.com/apache/thrift/pull/253#issuecomment-62168798
Jira issue created: https://issues.apache.org/jira/browse/THRIFT-2804
---
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] thrift pull request: Pull TAsyncProcessor interface out of TBaseAs...
Posted by hcorg <gi...@git.apache.org>.
Github user hcorg commented on the pull request:
https://github.com/apache/thrift/pull/253#issuecomment-62166688
please create issue on Jira and paste link to it here - that'll connect it to this requests
see: http://thrift.apache.org/docs/HowToContribute for details :)
BTW: couldn't TAsyncProcessor extend TProcessor?
public class TBaseAsyncProcessor<I> implements TAsyncProcessor, TProcessor
Processor word seems redundant ;)
---
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] thrift pull request: Pull TAsyncProcessor interface out of TBaseAs...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/thrift/pull/253
---
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] thrift pull request: Pull TAsyncProcessor interface out of TBaseAs...
Posted by hcorg <gi...@git.apache.org>.
Github user hcorg commented on the pull request:
https://github.com/apache/thrift/pull/253#issuecomment-62170947
yep, visitor would be best :) If you ever feel like doing such change don't hesitate ;)
I'll try to merge this request later today, thanks for Jira issue!
---
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] thrift pull request: Pull TAsyncProcessor interface out of TBaseAs...
Posted by m50d <gi...@git.apache.org>.
Github user m50d commented on the pull request:
https://github.com/apache/thrift/pull/253#issuecomment-62168094
It could, but ultimately they should become independent (or rather, there should be a superinterface that represents "either of these" and I guess a visitor rather than the instanceof check in AbstractNonblockingServer). TBaseAsyncProcessor and similar should not really implement the synchronous form of process() and should not really implement TProcessor at all (neither directly nor by dint of TAsyncProcessor extending it), but as I said in the initial pull request, that would be a more intrusive change; that said I'm happy to do that if you like.
---
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.
---