Merge lp://staging/~vila/uci-engine/delete-containers into lp://staging/uci-engine

Proposed by Vincent Ladeuil
Status: Needs review
Proposed branch: lp://staging/~vila/uci-engine/delete-containers
Merge into: lp://staging/uci-engine
Prerequisite: lp://staging/~vila/uci-engine/cleanup-data-store
Diff against target: 217 lines (+135/-9)
4 files modified
ci-utils/ci_utils/data_store.py (+44/-2)
ci-utils/ci_utils/testing/fixtures.py (+6/-1)
ci-utils/ci_utils/tests/test_data_store.py (+81/-2)
ci-utils/ci_utils/tests/test_fixtures.py (+4/-4)
To merge this branch: bzr merge lp://staging/~vila/uci-engine/delete-containers
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Para Siva (community) Approve
Review via email: mp+242316@code.staging.launchpad.net

Commit message

Safely delete swift containers.

Description of the change

This builds on top of https://code.launchpad.net/~vila/uci-engine/cleanup-data-store/+merge/242218 and extends the swift client retry policy to the delete container request.

This allows us to reliably delete swift containers whne they become useless (in-production tests, uci-britney, test themselves, etc).

I've tuned the overall timeout to ~5 minutes based on previous experimentations which showed real life inconsistency windows going up to 2 minutes.

/!\ logging and metrics are still missing ! This will be done in further MPs.

To post a comment you must log in.
909. By Vincent Ladeuil

One more test to ensure we fail after all retries (with a test reproducing the bug).

Tested with: ./run-tests ^tests.test_style ^tests.test_data_store ^ci-utils.*test_fixtures ^ci-utils.*test_data_store

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:909
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~vila/uci-engine/delete-containers/+merge/242316/+edit-commit-message

http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1732/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1732/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Para Siva (psivaa) wrote :

Looks ok, except for one inline comment

review: Needs Fixing
910. By Vincent Ladeuil

One more test for the retries=0 case to address review comment.

Revision history for this message
Vincent Ladeuil (vila) wrote :

Good catch !

I had added a test in revno 909 for that case but your remark made me realize I forgot one case, added as revno 910.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:910
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1734/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1734/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
Para Siva (psivaa) wrote :

Ok, thanks for fixing and the additional test. +1 from my side.

review: Approve
911. By Vincent Ladeuil

Fix pyflakes issue.

912. By Vincent Ladeuil

Fix pep8 issue.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:912
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1738/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/1738/rebuild

review: Approve (continuous-integration)

Unmerged revisions

912. By Vincent Ladeuil

Fix pep8 issue.

911. By Vincent Ladeuil

Fix pyflakes issue.

910. By Vincent Ladeuil

One more test for the retries=0 case to address review comment.

909. By Vincent Ladeuil

One more test to ensure we fail after all retries (with a test reproducing the bug).

Tested with: ./run-tests ^tests.test_style ^tests.test_data_store ^ci-utils.*test_fixtures ^ci-utils.*test_data_store

908. By Vincent Ladeuil

Extend swiftclient retry policy to container deletion with tests.

907. By Vincent Ladeuil

Meh, we want to test the real data store with a fake swift client, not the fake data store ;)

906. By Vincent Ladeuil

One less FIXME and a test plan.

905. By Vincent Ladeuil

Tested with ./run-tests ^tests.test_style ^tests.test_data_store befor submission.

(and some cosmetic changes)

904. By Vincent Ladeuil

This test pollutes the run output.

903. By Vincent Ladeuil

Log auth info (except for the password).

Rewrite exception in list_files.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches