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

Return success from publish command when document already has a published edition and no draft exists #3068

Closed
wants to merge 1 commit into from

Conversation

ryanb-gds
Copy link
Contributor

@ryanb-gds ryanb-gds commented Jan 2, 2025

Previously the publish command raised a conflict error in this case.

However, this error was rarely useful to the Publishing apps, as in almost all cases it is caused by publishing apps having to do their own dependency resolution due to shortcomings in Publishing API's dependency resolution, or by Sidekiq's "at least once" job running guarantees.

We therefore return a success response from the publish command and perform a no-op in this situation, rather than return an HTTP 409 error response for the publishing app to handle.

Why

This causes occasional errors in Whitehall which trigger Sentry alerts. Whilst we could ignore 409 responses in Whitehall, it seems sensible to make a change on the Publishing API side. From a RESTful API perspective the request has been successful in that the document is in the published state.

For further discussion see this Trello card: https://trello.com/c/h7xkpcUj and Slack thread

This will require an accompanying PR to update the Pact tests for Publishing API: alphagov/gds-api-adapters#1319

…shed edition and no draft exists

Previously the publish command raised a conflict error in this case.

However, this error was rarely useful to the Publishing apps, as in almost all cases it is caused by publishing apps having to do their own dependency resolution due to shortcomings in Publishing API's dependency resolution, or by Sidekiq's "at least once" job running guarantees.

We therefore return a success response from the publish command and perform a no-op in this situation, rather than return an HTTP 409 error response for the publishing app to handle.
@ryanb-gds
Copy link
Contributor Author

Closing this PR because, as @mike3985 pointed out, this is in fact a breaking change to Publishing API (hence the failing Pact test). I therefore need to either declare a new version of the publish endpoint, or make the change non-breaking somehow.

@ryanb-gds ryanb-gds closed this Jan 3, 2025
@ryanb-gds ryanb-gds deleted the idempotent-publishing branch January 3, 2025 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant