Details

Reviewers

olasd

Group Reviewers

Reviewers

Commits

rDICPff2b3e67853f: Add a check_vault command

Summary

It picks a random directory, requests its cooking from swh-web,
and waits for it to complete.

First step for T2126.

Depends on D2456 for CI to pass

Test Plan

The tests only check that the output conforms to Icinga's API to the best of my understanding of that API.

Diff Detail

Repository

rDICP Icinga plugins

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

vlorentz created this revision.Dec 13 2019, 4:27 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptDec 13 2019, 4:27 PM

Harbormaster failed remote builds in B9756: Diff 8667!Dec 13 2019, 4:27 PM

vlorentz added a child revision: D2454: Add a timeout to check_vault..Dec 13 2019, 4:27 PM

vlorentz edited the test plan for this revision. (Show Details)Dec 13 2019, 4:28 PM

vlorentz edited the summary of this revision. (Show Details)

vlorentz edited the summary of this revision. (Show Details)Dec 13 2019, 5:00 PM

vlorentz added a parent revision: D2456: Add swh-icinga-plugins.

I have the feeling that there's a way to factor out the state machine you're using for tests instead of duplicating it in all the tests.

I was about to point out that you should add a timeout, but you're adding that in a subsequent diff, so I guess that's fine.

swh/icinga_plugins/cli.py
28	I think we've usually used dashes rather than underscores here.
swh/icinga_plugins/vault.py
41	Does anything actually raise that exception?

This revision now requires changes to proceed.Dec 16 2019, 2:18 PM

In D2453#58116, @olasd wrote:

I have the feeling that there's a way to factor out the state machine you're using for tests instead of duplicating it in all the tests.

Yeah, it annoys me too, but I don't see a good way to do it

swh/icinga_plugins/cli.py
28	I was mirroring Icinga's doc instead of our conventions, but you're right.
swh/icinga_plugins/vault.py
41	oops, good catch. It will when I next update the diff