seldon-core icon indicating copy to clipboard operation
seldon-core copied to clipboard

fix: replace assert with if in egreedy router

Open malcolmk181 opened this issue 1 year ago • 1 comments
trafficstars

What this PR does / why we need it: This PR fixes an improper use of assert in the Epsilon Greedy Router. An assert is currently used to ensure that a passed in argument n_branches is greater than 0. However, if this code is running in production with the -O optimize flag, then this assert will be ignored by Python and potentially cause a later IndexError.

To replicate this issue, you can run the test suite for this file by cding into the components/routers/epsilon-greedy directory, then running pytest. The tests will pass as expected. However, if you run python -O -m pytest, one test will fail since the assertion is ignored, leading to an IndexError thrown by a later random.choice with less than 1 choice.

The code changes the assert to an if conditional that raises a ValueError explaining the mistake.

Which issue(s) this PR fixes: None

Special notes for your reviewer: Aside from the change in error type on line 13 of test_EpsilonGreedy.py, the changes are due only to black.

Changes EpsilonGreedy router to raise a ValueError instead of an AssertionError on an improper argument

malcolmk181 avatar Mar 26 '24 21:03 malcolmk181

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Apr 12 '24 14:04 CLAassistant