Fix retry decorator max substitution

[Reskov]

Resolves #207

[codecov-io]

Codecov Report

Merging #208 (a2d0085) into master (dc95053) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #208   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           73        73           
  Lines         2202      2202           
  Branches       308       308           
=========================================
  Hits          2202      2202           

Impacted Files	Coverage Δ
pypyr/dsl.py	100.00% <100.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 dc95053...a2d0085. Read the comment docs.

[yaythomas]

Excellent find, thank you so much @Reskov! 🙌

RetryDecorator sits at the very bottom of the decorator order of precedence, so in theory it's possible that the step itself might want to mutate the value (e.g have a different retry max on each iteration of a while loop). Edge case, I know!

But so in short, we shouldn't be over-writing the original value for self.max.

There's two potential routes here:

• add a max argument to exec_iteration and check that instead of self.max, and then in retry_loop do:

        if poll.while_until_true(interval=sleep,
                                 max_attempts=max)(
                self.exec_iteration)(context=context,
                                     step_method=step_method,
                                     max=max,
                                     ):  # pragma: no cover

or

• add a max_formatted attribute to self that gets formatted in retry_loop and then in exec_iteration do

 if self.max_formatted:
                if counter == self.max_formatted:

I'm not really too religious about which option is "better". The first option would mean updating a bunch of unit tests for exec_iteration, so the second is slightly lighter touch. What do you think?

[Reskov]

Yes, a good point, I was thought about it, but I have chosen the easiest one...

Personally, I like the first option with max as an extra argument to exec_iteration. It helps to keep the scope of the retry_decorator tidy.

[yaythomas]

It helps to keep the scope of the retry_decorator tidy.

good point! 👍 Big thank you as always for your excellent work!