Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C# extension methods are too limited for use in non-async code. #446

Closed
koolraap opened this issue Nov 3, 2015 · 5 comments
Closed

C# extension methods are too limited for use in non-async code. #446

koolraap opened this issue Nov 3, 2015 · 5 comments
Labels
Milestone

Comments

@koolraap
Copy link

koolraap commented Nov 3, 2015

The non-async methods generated in the C# FooBarExtensions.cs class are too limited for practical use.

  1. Cannot access custom headers. We pass an X-Correlation-Id between services, we can't do this with these methods. Please expose the customHeaders parameter in extension methods.
  2. Returning result.Body makes it impossible to retrieve the status code, which makes determining success or failure tricky, it also gimps our metric tracking.
public static FooBarModel GetDocument(this IFooBars operations, int? fooBarId)
{
    return Task.Factory.StartNew(s => ((IFooBars)s).GetDocumentAsync(fooBarId), operations, CancellationToken.None, TaskCreationOptions.None, TaskScheduler.Default).Unwrap().GetAwaiter().GetResult();
}

public static async Task<FooBarModel> GetDocumentAsync( this IFooBars operations, int? fooBarId, CancellationToken cancellationToken = default(CancellationToken))
{
    HttpOperationResponse<FooBarModel> result = await operations.GetDocumentWithHttpMessagesAsync(fooBarId, null, cancellationToken).ConfigureAwait(false);
    return result.Body;
}
@devigned
Copy link
Member

@stankovski thoughts on this?

@koolraap
Copy link
Author

The header/correlation-id thing can be worked around with something in the pipeline, but ultimately hiding any parameters makes sync code a second-class citizen to async code. Where I work we have 1% async code, most of our work is enhancements rather than greenfield. Moving to REST is a Big Thing, and that's partially repurposing existing sync code.

So, the question is, are you supporting sync code as a first class citizen?

@Nexusger
Copy link
Contributor

@koolraap If I see it correctly, your issue 1 also applies to the Async method, am I right? You can't inject custom header there either. Which is a bummer.

We have the same problem. Currently moving to micro services, we need to create an ever increasing number of services and the code created by Autorest is quite useful for that. Yet we have to modify the code every time we create it (expose custom headers to sync- and async- methods, to include a correlation ID).

Maybe another command line flag could be used, so the backward compatibility is kept? Something like
`...

-ExposeCustomHeader: Exposes the custom header attribute in the generated methods.

...`

@azuresdkci azuresdkci added the C# label Mar 1, 2016
@azuresdkci azuresdkci added this to the 0.16.0 milestone Mar 1, 2016
@tbombach tbombach modified the milestones: 0.16.0, 0.17.0 May 11, 2016
@John-Hart John-Hart removed this from the 0.17.0 milestone May 24, 2016
@stankovski
Copy link
Member

I would recommend adding a flag in autorest for C# to allow specifying what sync wrappers should be generated -SyncWrappers None|Essential|All. This will also be useful for clients where .Unwrap().GetAwaiter().GetResult(); is undesirable.

@koolraap
Copy link
Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants