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

Ensure API requests with ForceChangeset always flip creatingChangeset back off on failure #4988

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jkeiser
Copy link
Contributor

@jkeiser jkeiser commented Nov 18, 2024

This creates a new changeSetsStore.nonHeadApiRequest() function for API calls that force a new changeset to be created. It will flip creatingChangeSet on, and then flip it back off if the API fails. There are a number of instances where we flip it on, but don't flip it back off.

This should not change behavior in any instances where API requests succeed; boilerplate reduction here is entirely rote replacement (or should be--another set of eyes is welcome)

@github-actions github-actions bot added the A-web label Nov 18, 2024
@jkeiser jkeiser marked this pull request as draft November 18, 2024 20:33
@jkeiser
Copy link
Contributor Author

jkeiser commented Nov 18, 2024

Per @jobelenus there is some tricky stuff around where we keep track of the requests, and this PR would track these API requests in the ChangeSetsStore, which is probably not what we want (for places that externally track the requests). Converted to draft for the moment, and I'll think if there's a quick way to fix this before just scrapping.

@jkeiser jkeiser force-pushed the jkeiser/change-set-api-request branch from e5cb413 to 92fedca Compare November 18, 2024 22:03
Besides significantly decreasing boilerplate, this ensures we always flip
it back on failure (which we didn't always remember to do)
@jkeiser jkeiser force-pushed the jkeiser/change-set-api-request branch from 92fedca to 3a5cf07 Compare November 18, 2024 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants