search

kbase / kb_sdk

Build and test new apps for the KBase platform
http://kbase.github.io/kb_sdk_docs
MIT License
27 stars 32 forks source link

Sct-1366: Retry on check_job fail in base client #324

Closed JamesJeffryes closed 6 years ago

JamesJeffryes commented 6 years ago

This adds logic to retry the _check_job logic if a ConnectionError is raised by requests. This patch seems to work as demonstrated by kb_stringtie and ExpressionUtil modules.

The other change I am proposing here is to avoid compiling the module on docker build each time. I think it's pretty non intuitive that we commit the compiled code to the repo but that can get out of sync with the actual code run in the docker container because of this step.

MrCreosote commented 6 years ago

I'm pretty sure the catalog expects a compile on build so I don't think the Makefile change is going to work.

JamesJeffryes commented 6 years ago

FWIW, you can register and run a python module that does not compile in the dockerfile step. That's how I tested this patch out in stringtie. What happens with Perl or Java and if this is a good idea at all is something I'm willing to discuss

MrCreosote commented 6 years ago

So does the documentation in the catalog get built correctly without the compile report? And is the commit in the status() method correct?

JamesJeffryes commented 6 years ago

You are right, the catalog service gets the commit directly from the Server file so the compile is currently needed to ensure that that stays current.

JamesJeffryes commented 6 years ago

I reverted the change to Makefile as the compile is needed by catalog and that's not going to be trivial to change (having this dependency on kb-sdk in the base image is a major bummer though)

scanon commented 6 years ago

This looks good.