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

Does this library support dry-run? #288

Open
ruinshe opened this issue Nov 22, 2019 · 8 comments
Open

Does this library support dry-run? #288

ruinshe opened this issue Nov 22, 2019 · 8 comments

Comments

@ruinshe
Copy link

ruinshe commented Nov 22, 2019

I'm using this library for our internal project's command line tools, and I met a problem that we need a customized flag --dry-run to print all useful information but the request will not really sent.

Currently I added a util package to print the client object directly in client.OnBeforeRequest if the dry run mode is enabled then let the function return not nil error, and it seems not a good solution.

Do you have any suggestion to print the request information and stop sending API in this case?

Currently my solution:

if util.IsDryRunEnabled() {
    client = client.OnBeforeRequest(func(_ *resty.Client, request *resty.Request) error {
        fmt.Println("In dry-run mode:")
        fmt.Println(request)
        return errors.New("Dry run and interrupted.")
    })
}
@jeevatkm
Copy link
Member

jeevatkm commented Jan 8, 2020

@ruinshe I think it would be best to add the dry run option in the Request object. Having a flag in the request to drop the outgoing request.
E.g. client.R().DoDryRun()

If you're interested, you can submit PR otherwise I will take it up later on 😄

@ruinshe
Copy link
Author

ruinshe commented Jan 9, 2020

@jeevatkm Maybe I don't have much time in current stage, for free take it up..

Besides, I will take a look if I have time later.

@jeevatkm
Copy link
Member

jeevatkm commented Jan 9, 2020

@ruinshe Thanks, no worries. I will take it up later.

@jeevatkm jeevatkm added feature v2 For resty v2 labels Feb 18, 2020
@lrita
Copy link
Contributor

lrita commented Apr 30, 2020

I think we have SetTransport() is enough to support this. I think there alway some customized-codes need be wrote by users, it does not necessary to support all non-common use cases.

@jeevatkm
Copy link
Member

@lrita has a point. Technically custom transport could be used with user code to achieve this.

For now I will put this in the backlog.

@ruinshe
Copy link
Author

ruinshe commented May 11, 2020

Thank all for the suggestion, although the solution seems to be same as what I did, it will make th code more clearer.

I switched my code using customized transport for request rewritten.

@jeevatkm
Copy link
Member

@ruinshe Do you mind sharing the code what you did? so that we can evaluate to include in the resty lib or not?

If it's trivial, we can add it as resty documentation. What do you think?

@ruinshe
Copy link
Author

ruinshe commented May 11, 2020

@ruinshe Do you mind sharing the code what you did? so that we can evaluate to include in the resty lib or not?

If it's trivial, we can add it as resty documentation. What do you think?

I think refine the document helps, because in the transport we can do everything (even if the user only cares about the headers not whole request, etc).

@jeevatkm jeevatkm removed the v2 For resty v2 label Sep 12, 2021
@jeevatkm jeevatkm added this to the v3.0.0 Milestone milestone Oct 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants