Auditor accounting plugin

[stefan-k]

This is so far only a draft. I will request a review when ready. Comments are welcome though.

This uses the python-auditor package (which provides the pyauditor module). This packages has no dependencies itself, but in order to properly deal with timezones, pytz and tzlocal are required.

Action plan:

• Plugin logic
  • Blindly implement and hope for the best
  • Actually give it a try
  • Redo everything
• Tests
  • Copy-Paste from another plugin
  • Adapt to new plugin
• Documentation
  • Code
  • Docs

Edit: pyauditor is written in Rust and as such requires python>=3.7

Edit2: Works with python>=3.6 now (at least on macos and linux, not on windows)

[giffels]

Thanks a lot for your contribution. Unfortunately, we currently plan to support Python 3.6 as long as Centos 7 is around or EPEL decides to provide a more recent version. Is there any chance that python-auditor can support Python 3.6?

[stefan-k]

The library for the glue code (PyO3) only supports 3.7 and up in current versions. Older versions apparently support 3.6 as well, so I'll have to see if I can feature gate this somehow...

[codecov-commenter]

Codecov Report

Merging #263 (bc39f2e) into master (cc13dfd) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #263      +/-   ##
==========================================
+ Coverage   98.81%   98.85%   +0.03%     
==========================================
  Files          54       55       +1     
  Lines        2192     2262      +70     
==========================================
+ Hits         2166     2236      +70     
  Misses         26       26              

Impacted Files	Coverage Δ
tardis/plugins/auditor.py	100.00% <100.00%> (ø)
tardis/utilities/utils.py	100.00% <0.00%> (ø)
tardis/resources/poolfactory.py	100.00% <0.00%> (ø)
tardis/plugins/sqliteregistry.py	100.00% <0.00%> (ø)
tardis/resources/dronestates.py	99.35% <0.00%> (+0.02%)	⬆️
tardis/resources/drone.py	98.96% <0.00%> (+0.15%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[stefan-k]

I'm having problems with mocking and I was hoping that one of you may be able to help me.
I'm using a builder pattern to create an AuditorClient from an AuditorClientBuilder via .build(). I managed to mock AuditorClientBuilder but somehow I am unable to mock AuditorClient. I also tried mocking the entire pyauditor module but failed.

The tests complain about await:

        if isinstance(state, AvailableState):
            record = self.construct_record(resource_attributes)
>           await self._client.add(record)
E           TypeError: object MagicMock can't be used in 'await' expression

I am aware of async_return() but I think I'm missing something way more fundamental.

Any help is highly appreciated :)

[giffels]

Unfortunately, you have to mock the entire call stack. For example in test_notify.

self.mock_auditorclientbuilder.return_value.address.return_value.timeout.return_value.build.return_value.add.return_value = async_return()

run_async(
    self.plugin.notify, state=AvailableState(), resource_attributes=test_param
)

Of course you should potentially spilt that line in several once and maybe do it in the setUp method.

[stefan-k]

Unfortunately, you have to mock the entire call stack.

Thanks, I would ever have come up with that myself. This works fine.

Of course you should potentially spilt that line in several once and maybe do it in the setUp method.

Black thinks that ridiculously long line is just fine. I can only split it using temporary variables:

builder = self.mock_auditorclientbuilder.return_value.
builder = builder.address.return_value.timeout.return_value
client = builder.build.return_value
client.add.return_value = async_return()

I hope that's acceptable too.

[RHofsaess]

👍
Only some text comments, see inline.

[stefan-k]

Thanks for the review @RHofsaess ! I updated the docs as suggested and added documentation for the construct_record method as well.

[stefan-k]

The CI fails at code unrelated to the PR. Maybe just a glitch, but I don't have the authority to restart the failed jobs.

[maxfischer2781]

Looks like a glitch indeed. I've rerun the tests now, looks good. 👍

[RHofsaess]

👍

[giffels]

Thanks a lot for your work. I have some minor comments inline. Could you address them, please?

[stefan-k]

Thanks a lot for the review @giffels !

EDIT: Not sure what I did with requesting new reviews but somehow requesting a review from one person removes the request from another person ;)

[giffels]

LGTM! Thanks a lot.

[RHofsaess]

👍