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

fix: always delete all backups from an owned test instance #557

Merged
merged 2 commits into from Oct 27, 2020

Conversation

olavloite
Copy link
Collaborator

An instance can only be deleted once all backups on the instance have been deleted. The integration test environment should therefore first delete all backups on an owned instance before trying to delete the instance itself.

Individual test cases are responsible for deleting any backups that are created during the test, but sometimes backup creation operations do not finish within reasonable time. The deletion of these backups by the individual test cases sometimes seem to fail. This change is an additional fallback for making sure that all backups that were created during the tests are removed, and that the test instance can be removed.

Fixes #542

An instance can only be deleted once all backups on the instance have been
deleted. The integration test environment should therefore first delete all
backups on an owned instance before trying to delete the instance itself.

Fixes #542
@olavloite olavloite requested a review from a team as a code owner October 27, 2020 14:28
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 27, 2020
@codecov
Copy link

codecov bot commented Oct 27, 2020

Codecov Report

Merging #557 into master will increase coverage by 0.10%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #557      +/-   ##
============================================
+ Coverage     83.93%   84.03%   +0.10%     
- Complexity     2501     2503       +2     
============================================
  Files           141      141              
  Lines         13766    13806      +40     
  Branches       1315     1317       +2     
============================================
+ Hits          11554    11602      +48     
+ Misses         1667     1657      -10     
- Partials        545      547       +2     
Impacted Files Coverage Δ Complexity Δ
...gle/cloud/spanner/AsyncTransactionManagerImpl.java 70.42% <0.00%> (-1.01%) 12.00% <0.00%> (ø%)
...ain/java/com/google/cloud/spanner/SessionPool.java 88.13% <0.00%> (+1.23%) 71.00% <0.00%> (ø%)
...ud/spanner/SessionPoolAsyncTransactionManager.java 86.55% <0.00%> (+1.64%) 11.00% <0.00%> (+2.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d92b7c2...5cf721e. Read the comment docs.

Copy link
Contributor

@hengfengli hengfengli left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for fixing this issue.

Copy link
Contributor

@thiagotnunes thiagotnunes left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@thiagotnunes thiagotnunes merged commit ff571b0 into master Oct 27, 2020
@thiagotnunes thiagotnunes deleted the delete-backups-from-test-instance branch October 27, 2020 22:52
ansh0l pushed a commit to ansh0l/java-spanner that referenced this pull request Nov 10, 2022
This is an auto-generated regeneration of the .pb.go files by
cloud.google.com/go/internal/gapicgen. Once this PR is submitted, genbot will
update the corresponding PR to depend on the newer version of go-genproto, and
assign reviewers. Whilst this or any regen PR is open in go-genproto, genbot
will not create any more regeneration PRs. If all regen PRs are closed,
gapicgen will create a new set of regeneration PRs once per night.

If you have been assigned to review this PR, please:

- Ensure that CI is passing. If it's failing, it requires your manual attention.
- Approve and submit this PR if you believe it's ready to ship. That will prompt
genbot to assign reviewers to the google-cloud-go PR.

Corresponding google-cloud-go PR: googleapis/google-cloud-go#3849

Changes:

feat(monitoring/apiv3): Added support for Monitoring Query Language: https://cloud.google.com/monitoring/mql feat: Added support for units in the MetricService feat: Added total_size to the response of ListAlertPolicies. fix: Un-deprecated cluster_istio for service monitoring. feat: Added IstioCanonicalService for service monitoring. feat: Added creation and mutation records to notification channels. feat: Added support for querying metrics for folders and organizations. fix: Extended the default deadline for UpdateGroup to 180s. feat: Added support for secondary aggregation when querying metrics.
  PiperOrigin-RevId: 364422694
  Source-Link: googleapis/googleapis@2124b62

chore: upgrade gapic-generator-python to 0.43.1
  PiperOrigin-RevId: 364411656
  Source-Link: googleapis/googleapis@149a3a8

feat: Add TAXI travel mode and PolylineEncoding.
  PiperOrigin-RevId: 364401367
  Source-Link: googleapis/googleapis@961dc7c

feat: added v3beta1 proto for online and batch document translation
  PiperOrigin-RevId: 364358156
  Source-Link: googleapis/googleapis@d6b4fb3

docs(dialogflow): fix link for version and environment in participant doc. fix: Add all missing regional HTTP bindings in DF API.
  PiperOrigin-RevId: 364356707
  Source-Link: googleapis/googleapis@5ef92c3

fix!(dialogflow): Remove resource_reference from UpdateAnswerRecord.
  Committer: @sheimi
  PiperOrigin-RevId: 363947872
  Source-Link: googleapis/googleapis@5da78db
rajatbhatta pushed a commit to rajatbhatta/java-spanner that referenced this pull request Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spanner.it.ITBackupTest: testBackups failed
3 participants