Merge lp:~cprov/britney/testclient-api into lp:~canonical-ci-engineering/britney/queued-announce-and-collect

Proposed by Celso Providelo
Status: Merged
Merged at revision: 435
Proposed branch: lp:~cprov/britney/testclient-api
Merge into: lp:~canonical-ci-engineering/britney/queued-announce-and-collect
Diff against target: 473 lines (+446/-0)
3 files modified
britney.py (+57/-0)
testclient.py (+178/-0)
tests/test_testclient.py (+211/-0)
To merge this branch: bzr merge lp:~cprov/britney/testclient-api
Reviewer Review Type Date Requested Status
Para Siva (community) Approve
Canonical CI Engineering Pending
Review via email: mp+259668@code.launchpad.net

Commit message

Initial implementation of a generic interface for proposed-migration (britney) to produce new-candidates events and consume test results from the CI-infrastructure.

Description of the change

Initial implementation of a generic interface for proposed-migration (britney) to produce new-candidates events and consume test results from the CI-infrastructure.

To post a comment you must log in.
Revision history for this message
Para Siva (psivaa) wrote :

Thanks a lot cprov for the MP.
I really like your way of handling the cache. Making keys with name and version is a clever way. And thanks for adding tests too.

I have a few points,
1. To include the conf file changes,
2. An inline comment in avoiding duplicate entries in the reg
3. I think the cleanup will handle entries with one source having two or more versions at the same time, but want to double check.
4. Also i'm not sure if we're handling acking and nacking etc. I'm asking this because we're removing entries from the cache and that may lead to messages being in the queue forever?

review: Needs Fixing
Revision history for this message
Celso Providelo (cprov) wrote :

Psivaa,

Thanks for the review.

Regarding config-changes, they will be only necessary when we do integration tests (which I've mentioned are missing, on purpose, from this MP).
On point 2) I don't get the registry duplication, it's impossible, since it's a dictionary, perhaps you mean re-announcement ?
On point 3) take a look at the corresponding test, it may clarify things about cleanup() effects.
On message ack-ing, you are right it was missing in the collect() action, fixed.

[]

Revision history for this message
Para Siva (psivaa) wrote :

Thanks for fixing the comments cprov.

> On point 2) I don't get the registry duplication, it's impossible, since it's
> a dictionary, perhaps you mean re-announcement ?
>
Point 2 was about a notification about my inline comment, not using make_key bit, which you've fixed.

And thanks a lot again for this MP. Whole lot different than what I proposed.

review: Approve
lp:~cprov/britney/testclient-api updated
442. By Celso Providelo

addressing review comments

443. By Celso Providelo

extend existing tests for ignored test results (series mismatch).

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