search

pact-foundation / pact_broker-client

A Ruby and CLI client for the Pact Broker. Publish and retrieve pacts and verification results.
MIT License
69 stars 47 forks source link

Do not retry when --dry-run (or PACT_BROKER_CAN_I_DEPLOY_DRY_RUN) is provided #149

Closed stan-is-hate closed 3 months ago

stan-is-hate commented 1 year ago

If you run can-i-deploy in dry-run mode but provide --retry-while-unknown value, the command would retry on the unknown verification results, same as without --dry-run.

I'd argue that if you're running in dry-run, you're not interested in the results anyway, so there's no point waiting for the verification results to become published. Moreover, if you're running in dry run, there's a high chance that the verification result will never become available, since dry-run use cases include "break glass" situations (where you don't care and just want to deploy) or onboarding your systems to pact, where some services might delay the onboarding for whatever reason, so they won't upload any verifications.

So my suggestion would be to ignore --retry-while-unknown and --retry-interval flags if --dry-run is provided.

In fact, that's how --ignore/PACT_BROKER_CAN_I_DEPLOY_IGNORE already is, if you run can-i-deploy with both --ignore and --retry.. flags provided, it won't retry.

Alternatively, if you folks want to keep dry run as a true dry run, how about providing a way to ignore all pacts? Either --ignore-all/PACT_BROKER_CAN_I_DEPLOY_IGNORE_ALL or --ignore="*" or something?

Thanks! I'd be happy to contribute, but have zero ruby knowledge.

stan-is-hate commented 1 year ago

friendly ping on this one :)

bethesque commented 9 months ago

Thanks for the suggestion. I can see the logic of your proposal. For now, can you please handle this in the script that calls the can-i-deploy, as I will not get to this issue any time soon unfortunately.

YOU54F commented 3 months ago

Released in 1.76.0

Will be propagating through to the standalone and docker image shortly

stan-is-hate commented 2 months ago

Thanks for fixing this! Is it possible to do the same for can-i-merge please?

YOU54F commented 2 months ago

It should work for can-i-merge as it calls CanIDeploy

https://github.com/pact-foundation/pact_broker-client/blob/9fdb861bd8a4a21166ef39c15b559a50682fd7ae/lib/pact_broker/client/cli/matrix_commands.rb#L57-L70