awx icon indicating copy to clipboard operation
awx copied to clipboard

Move the IS_TESTING method out of settings

Open AlanCoding opened this issue 3 years ago • 1 comments

SUMMARY

It is really weird to find this function definition inside of settings, firstly. It also comes with some cost - as I looked into the logic for taking out settings snapshot and IS_TESTING gets put into the snapshot by the copy.deepcopy and it is the only thing that isn't a plain int/string/bool type, or a list/dict of those types.

Doing some historical analysis, I found that the introduction corresponded to a special postprocess.py script for development.

https://github.com/ansible/awx/commit/60224cdbe4845f2e80b38486b62fd8f5feedfca0#diff-bd08a936903e8f4f8830bf6d0caa6ff3520d75ec50a62c594b32c4a33e800912

This makes sense, and it would be why you would introduce a method like this in settings. However, no trace of that remains today.

All of the uses I see now seem to actually be relatively modern, as new code found this method and applied it in an ordinary type place. These uses have no need for such an unorthodox definition inside of settings, and there are oodles of reasons to be concerned with all the layers of wrapping we have on top of settings.

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • API
ADDITIONAL INFORMATION

This is motivated by @shanemcd 's recent work to clean up settings.

For merging, one obvious criteria will be that the tests pass, because this is mostly important as a metric for turning things off when tests are running. The other thing we want is to make sure that nothing obvious breaks.

AlanCoding avatar Sep 14 '22 13:09 AlanCoding

I tried to measure how much speed boost this was compared to before (due to the deepcopy) and it was like 0.005 seconds. So nothing to get excited about.

AlanCoding avatar Sep 14 '22 14:09 AlanCoding