Update module_test_python_client.vm.properties

[bio-boris]

See #347
Called at https://cs.github.com/kbase/kb_sdk/blob/07a0a3ce72060510b58202ce8151f4d4818a81aa/src/java/us/kbase/mobu/initializer/ModuleInitializer.java#L152
and
https://cs.github.com/kbase/kb_sdk/blob/07a0a3ce72060510b58202ce8151f4d4818a81aa/src/java/us/kbase/mobu/compiler/TemplateBasedGenerator.java#L206

[MrCreosote]

Why are there almost 100 checks? That seems insane

[MrCreosote]

Making changes to kb_sdk makes me very nervous due to the centrality of the module in KBase, its complexity, and the fact that the tests are unrunnable (except maybe on my magic VM, but IIRC they weren't working the last time I tried, which was years ago).

I assume that whatever change broke things, and caused this change to be necessary, got through because of the lack of tests.

The change itself looks reasonable at face value. The questions I have are

• How and where did this bug get introduced?
• What testing has been done to make sure the change works and is backwards compatible?

[bio-boris]

1. I did not investigate how and when this bug was introduced
2. As for testing, I did it late at night and should probably redo it, but the testing I did was

• built the image with docker build . -t kbase-sdk:0.2.6-dev and updated my local /bin/kb-sdk to use this new built sdk

• deleted the modules server file, and ran make to regenerate it and see if it generated the tests correctly

• TODO is to create a new module using this new kb-sdk

• TODO: Play with this initPythonPackages(pythonServerPath, output, false); to see if that's the issue or not?

• As for being backwards compatible, I think it will NOT be backwards compatible with this fix, but all versions since the bug introduced and have been "released" (but not pushed to dockerhub) have this issue as well..
  The way to may it backwards compatible will be to in addition to this fix, make the kb-sdk copy the clients into the correct location and/or update the makefile to correctly generate a testing script with the right PYTHONPATH..

(I'm not entirely sure if its possible to change paths and move things around and would need to look into further. Making these changes in the MAKEFILE is not a portable solution as people may have updated the makefile..)

[MrCreosote]

Can we figure out how the bug came to be? That'll also help out re figuring out whether the change is backward compatible or not. It'd be bad if we needed to recompile an old module and it breaks

[bio-boris]

Looking at the git blame, the bug came to be when it got added to the template

3fe3e88#diff-c4bd14f7c977da591d617fdf8e484fd558391135ad1607871c93b549cff53875R19

Hmm, the other one got added https://github.com/kbase/kb_sdk/blob/3fe3e8875bccc2d97ddcca802fede3ff00f506b0/src/java/us/kbase/templates/module_test_python_client.vm.properties way back though so maybe it was during the python2-3 update? Not sure.

I think these changes actually make it more backwards compatible.

Wait this wouldn't affect recompiling old modules, because these steps only happen during init of the module I believe.

[MrCreosote]

It looks to me that whenever the change was made to put all the clients into installed_clients the templates weren't updated correctly and still pointed to the old locations. I think both the commits above are prior to that

[MrCreosote]

One of the files is the python server template; I'm pretty sure that gets recompiled (retemplated?) every time make compile is run

[MrCreosote]

For testing you say you should probably rerun the tests, maybe when you do that you could try recompiling a really old module like DFU and seeing if it still works, recompiling a newer module (not sure what's new) and seeing it it still works, and making a new module and checking it works, along with the tests you outlined above

[ialarmedalien]

I have been annoyed by this bug for several years now!

[ialarmedalien]

I was looking at the "insane" GHA test failures in another kb-sdk PR. Some of the failures are due to GHA issues (timeouts, etc.), but there are also problems with the example module that gets created as part of the GHA tests, specifically that the tool it installed (something from sourceforge) is no longer available.