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

ExecutePatchtAsync #1948

Closed
iwtu opened this issue Oct 26, 2022 · 10 comments · Fixed by #2050
Closed

ExecutePatchtAsync #1948

iwtu opened this issue Oct 26, 2022 · 10 comments · Fixed by #2050

Comments

@iwtu
Copy link

iwtu commented Oct 26, 2022

Is not possible to call method PATCH wihout throwing an exception when request was not successful.
Basically I'm missing the same funcionality as there is for GET, POST, PUT and DELETE methods.

@alexeyzimarev
Copy link
Member

I bet you just need some overloads. Please feel free to open a PR to add them. These are basically copy-pasting from those you mentioned, like this:

    public static Task<RestResponse> ExecutePostAsync(this RestClient client, RestRequest request, CancellationToken cancellationToken = default)
        => client.ExecuteAsync(request, Method.Post, cancellationToken);

I would say that "impossible" is an overstatement as ExecuteAsync(request, Method.Patch) will do exactly what you want. Those extensions are just adding syntactic sugar over the base API.

@sebastienlabine
Copy link

Same goes for ExecuteDeleteAsync.

@alexeyzimarev
Copy link
Member

I would greatly appreciate it if you guys submit PRs adding two LoC plus the XML comment for each of those missing extensions.

@akshay-zz
Copy link

akshay-zz commented Dec 27, 2022

@alexeyzimarev Can I add them ExecuteDeleteAsync and ExecutePatchtAsync? I would be happy to contribute. Also is test cases required for this?

@iwtu
Copy link
Author

iwtu commented Dec 28, 2022

To be honest, I would be happy to:

  • remove Execute{METHOD}Async

OR

  • Execute{METHOD}Async will override client behaviour.

It happened to me few times, that I used on request ExecutePostAsync but client for set to GET and it was really hard for me to notice that, because server behaved somehow correctly.

@alexeyzimarev
Copy link
Member

It happened to me few times

Could you elaborate? Do you mean you created a GET request and used ExecutePostAsync?

@alexeyzimarev
Copy link
Member

@alexeyzimarev Can I add them ExecuteDeleteAsync and ExecutePatchtAsync? I would be happy to contribute. Also is test cases required for this?

Of course

@iwtu
Copy link
Author

iwtu commented Dec 29, 2022

It happened to me few times

Could you elaborate? Do you mean you created a GET request and used ExecutePostAsync?

yes. Just copy & paste * change only Execute. It happened to me pretty often.

@akshay-zz
Copy link

akshay-zz commented Jan 2, 2023

@alexeyzimarev #1996 please review the PR.

@alexeyzimarev
Copy link
Member

Fixed in #1996

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