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

Wait for last operation if not completed #59

Merged
merged 2 commits into from
Nov 27, 2024

Conversation

stephen-soltesz
Copy link
Contributor

@stephen-soltesz stephen-soltesz commented Nov 26, 2024

The Autojoin API deployment includes three distinct steps in production:

  1. deploy the autojoin App Engine service
  2. deploy the openapi endpoints API
  3. deploy the dispatch.yml to direct requests to this service for custom domain names

While step 2 will return successfully, the operation it creates is not yet "COMPLETE" by the time step 3 begins, which generates a build error like the one below.

This change adds an explicit operation wait on that operation before continuing to step 3.

Build error:

Step #2: ERROR: (gcloud.app.deploy) Apps instance [mlab-autojoin] is the
subject of a conflict: Cannot operate on apps/mlab-autojoin because an
operation is already in progress for apps/mlab-autojoin by e309c37c-1c30-4074-9c06-4805a0935b05.

This change is Reviewable

@coveralls
Copy link
Collaborator

coveralls commented Nov 26, 2024

Pull Request Test Coverage Report for Build 12039568963

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 98.92%

Totals Coverage Status
Change from base Build 12038119053: 0.0%
Covered Lines: 1191
Relevant Lines: 1204

💛 - Coveralls

@stephen-soltesz stephen-soltesz removed the request for review from nkinkade November 26, 2024 21:41
@stephen-soltesz
Copy link
Contributor Author

Not ready for review yet

@stephen-soltesz
Copy link
Contributor Author

Okay - PTAL?

Copy link
Contributor

@nkinkade nkinkade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reminded me that I had hit this very same issue earlier this year:

https://github.com/m-lab/visualizations/pull/10/files

I had tried to use the operations wait functionality, but there was a bug in it and I just fell back on using sleep with a safe amount of time. Your implementation is very similar to what I had originally in the PR above, but I found that sometimes the command substitution didn't return anything due to likely very tight timings, and when that happens the entire command fails, and the build fails too. I'm wondering if your solution might run into the same issue?

Reviewable status: 0 of 1 approvals obtained

Copy link
Contributor Author

@stephen-soltesz stephen-soltesz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, it definitely could - the || : was meant to catch the case where the wait operation fails, either for the reason you describe or any other.

What do you think?

Reviewable status: 0 of 1 approvals obtained

Copy link
Contributor

@nkinkade nkinkade left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Sorry I missed that bit. LGTM.

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained

@stephen-soltesz stephen-soltesz merged commit caf7e93 into main Nov 27, 2024
8 checks passed
@stephen-soltesz stephen-soltesz deleted the sandbox-soltesz-operation-wait branch November 27, 2024 00:23
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.

3 participants