Fix missing filters for get settings validator #5480
Conversation
✅ Deploy Preview for inventree canceled.
|
wolflu05
added
bug
|
@wolflu05 please check out the CI errors |
|
Oh, I see, that is not the correct fix for my issue, will have to investigate why the clean function sometimes get the correct kwargs and sometimes not. |
|
I didn't manage to identify why I sometimes not get the kwargs but in the tests I get them. This is the easiest fix that works. Do you have a better idea of fixing the validators for plugins? |
|
@wolflu05 happy with this fix, but given this might change some pretty low-level stuff, can you please provide some updated unit tests? In particular related to the edge cases you were seeing |
|
@wolflu05 would you be able to throw in some extra unit tests here? |
|
@SchrodingersGat I now found the place where the clean function is called with additional kwargs which is against Django's implementation, so I removed the kwargs completely and replaced them with the |
|
These types of errors are always very frustrating to track down! Thanks for adding the tests |
|
@SchrodingersGat I found the culprit. I came across this after connecting via tmate and adding several prints to the code while running the tests over and over and see how far it runs. I then discovered that the validator doesn't run because it's not callable, but it is a static method which is strange. I then googled why static methods are sometimes not callable and found this. I then asked myself why it works on my machine and on ci not and then found out, I run python 3.10 in the devcontainer and ci is using 3.9. Python must have changed that behavior. I can validate that by running the provided snippet in the StackOverflow post. On my machine with 3.10 it returns True for both cases and on ci only for the direct check. So TL;DR, make sure we don't use static methods for validators anywhere, as they are not marked as callable in some circumstances. The tests should pass now after moving the validator into the global scope as a normal function, so you can merge this finally. |
|
@wolflu05 that's some fine detective work, well done! |
💔 All backports failed
Manual backportTo create the backport manually run: Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
* Fix missing filters for get settings validator * merge default model instance filters and kwargs * Added tests for validators * Give it a try without the kwargs passed to clean in save function * Added string for identification for debug statement * Added more debug comments * Added more debug prints * Fix test debug * Modiefied workflow * trigger ci * Fix test and remove unused kwargs * Added debug prints * Only run one test in ci * Added more debug code * Remove all debug prints and reset workflow * Reset overlooked file (cherry picked from commit 9a6c2d2) # Conflicts: # InvenTree/plugin/samples/integration/test_sample.py
* Fix missing filters for get settings validator (#5480) * Fix missing filters for get settings validator * merge default model instance filters and kwargs * Added tests for validators * Give it a try without the kwargs passed to clean in save function * Added string for identification for debug statement * Added more debug comments * Added more debug prints * Fix test debug * Modiefied workflow * trigger ci * Fix test and remove unused kwargs * Added debug prints * Only run one test in ci * Added more debug code * Remove all debug prints and reset workflow * Reset overlooked file (cherry picked from commit 9a6c2d2) # Conflicts: # InvenTree/plugin/samples/integration/test_sample.py * Add missing import * Added second missing import
This PR fixes the missing filters to get the settings validators for plugins to work again.