Github user adam-resdiary commented on the pull request:
https://github.com/apache/thrift/pull/762#issuecomment-170243463
@nsuke Sorry if I'm misunderstanding you, but I don't think I explained
myself very well with
> This doesn't really make sense on the server side since if you implement
a service you end up leaving all the async method unimplemented.
Here's an example scenario just so it's clear what I'm talking about:
Say I have two servers, Server 1 and Server 2. I also have two thrift
services, Thrift Service A and Thrift Service B. Server 1 implements Thrift
Service A, and calls Thrift Service B on Server 2. Server 2 implements Thrift
Service B. So something like this:
| Server 1 | Server 2 |
|------------|-------------|
| Implements Thrift Service A | Implements Thrift Service B |
| Calls Thrift Service B | |
Here's example thrift definitions:
```
service ThriftServiceA
{
void Method1();
}
service ThriftServiceB
{
string GetSomeInformation();
}
```
I then generate the code using the thrift compiler, and use the async:true
option. Because of the way we generate the code for our solutions, this causes
the async interfaces to be generated for both Thrift Service A, and Thrift
Service B.
On Server 1, I then have some kind of API call that calls Thrift Service B:
```csharp
[Route("api/Information")]
public async Task<string> GetSomeInformation()
{
return await _thriftServiceB.GetSomeInformationAsync();
}
```
This makes perfect sense because I'm making an async call to a thrift
service via the thrift client.
I then go to implement Thrift Service A, and what I end up with is
something like:
```csharp
public class ThriftServiceAImpl : ThriftServiceA.Iface
{
public void Method1()
{
// Do something
}
public Task Method1Async()
{
throw new NotImplementedException("There's no point implementing
this");
}
}
```
So in the example above, there's no point implementing Method1Async,
because the current HttpHandler implementation doesn't ever call it - it only
calls the synchronous implementation. So it's just noise, but you have to
implement it because it's part of the interface.
So my patch doesn't actually change the method names at all, it just splits
the two sets of methods into two separate interfaces so that if you're only
implementing the synchronous set you only need to implement the synchronous
methods, and the same for the async ones.
Now that I think about it, I think my change is actually backwards
compatible, because Iface implements ISync and IAsync, so there's probably no
need for a separate compiler flag. I was just playing it safe. I'll leave that
decision up to you guys though.
---
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 [email protected] or file a JIRA ticket
with INFRA.
---
