Skip to content

Conversation

@m50d
Copy link

@m50d m50d commented Nov 7, 2014

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.

@hcorg
Copy link
Member

hcorg commented Nov 7, 2014

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 implements TAsyncProcessor, TProcessor
Processor word seems redundant ;)

@m50d
Copy link
Author

m50d commented Nov 7, 2014

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.

@m50d
Copy link
Author

m50d commented Nov 7, 2014

Jira issue created: https://issues.apache.org/jira/browse/THRIFT-2804

@hcorg
Copy link
Member

hcorg commented Nov 7, 2014

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!

@asfgit asfgit closed this in fed887f Nov 7, 2014
stigsb pushed a commit to stigsb/thrift that referenced this pull request Nov 28, 2014
…ncProcessor

Client: Java
Patch: Michael Donaghy

This closes apache#253
@bsideup
Copy link
Member

bsideup commented Apr 25, 2015

👍

allengeorge pushed a commit to allengeorge/thrift that referenced this pull request Jan 1, 2017
…ncProcessor

Client: Java
Patch: Michael Donaghy

This closes apache#253
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants