kbase / JobRunner

KBase Job Runner
MIT License
0 stars 5 forks source link

call get_ip only when necessary #85

Closed Tianhao-Gu closed 5 months ago

Tianhao-Gu commented 5 months ago

Ensure that get_ip() is not called if the environment variable CALLBACK_IP is already set, as invoking get_ip() may lead to errors in environments (Argonne sequoia) with restricted outbound traffic.

bio-boris commented 5 months ago

Hi! What is the context for this change? Can you please link update the description please? Are there any testing instructions for this change? I think we should set up coverage as well and ensure that future changes are unit tested. Thanks!

Tianhao-Gu commented 5 months ago

Hi! What is the context for this change? Can you please link update the description please? Are there any testing instructions for this change? I think we should set up coverage as well and ensure that future changes are unit tested. Thanks!

👍 added tests and desc.

bio-boris commented 5 months ago

Thanks! Glad to hear that these latest changes work on Sequoia.

Can you please check out the tests and see what is going on with them? I believe it says that they are running, but if you look at the logs, it looks like they are failing. Are they indeed failing, or are the tests expected to fail that way?

Tianhao-Gu commented 5 months ago

Thanks! Glad to hear that these latest changes work on Sequoia.

Can you please check out the tests and see what is going on with them? I believe it says that they are running, but if you look at the logs, it looks like they are failing. Are they indeed failing, or are the tests expected to fail that way?

All tests passed. Some tests might just test failures. I reviewed some past GHA test runs, and the test logs are structured like this.

Ran 33 tests in 42.356s

OK