USDA-ARS-NWRC / awsm

Automated Water Supply Model (AWSM) was developed at the USDA Agricultural Research Service. AWSM was designed to streamline the workflow used to forecast the water supply of multiple water basins.
Other
9 stars 2 forks source link

Interface - run_isnobal - Improve subprocess call. #62

Closed jomey closed 4 years ago

jomey commented 4 years ago

Wait until the subprocess has finished and don't exit if it hasn't returned anything.

Got this warning couple of times now, when running the test suite:

ResourceWarning: subprocess 50139 is still running
  ResourceWarning, source=self)
ResourceWarning: Enable tracemalloc to get the object allocation traceback
/Volumes/warehouse/projects/ARS/AWSM/awsm/framework/framework.py:451: ResourceWarning: unclosed file <_io.BufferedReader name=5>
  smin.run_isnobal(self, offset=offset)
ResourceWarning: Enable tracemalloc to get the object allocation traceback
/Volumes/warehouse/projects/ARS/AWSM/awsm/framework/framework.py:451: ResourceWarning: unclosed file <_io.BufferedReader name=7>
  smin.run_isnobal(self, offset=offset)
ResourceWarning: Enable tracemalloc to get the object allocation traceback
scotthavens commented 4 years ago

Like Scott I know we have done this in katana so I wasn't totally sure the differences in the the methods. So I ran a little test to see how katanas approach and this approach would log something ongoing using the linux command sl ( if you dont know what it is, check it out...its fun!)

I also tested it with the output from inicheck just to see something more familiar.

The only differences I can see besides better exception catching here is that this doesn't strip the output. So extra line returns were showing up in the output of sl and 'inicheck'.

In Katana I had to do a rstrip() on the line to remove the new line, otherwise it looked odd. But good to know this is handling errors better, I will make similar changes in Katana while I'm at it today.

jomey commented 4 years ago

In Katana I had to do a rstrip() on the line to remove the new line, otherwise it looked odd. But good to know this is handling errors better, I will make similar changes in Katana while I'm at it today.

This is long term thinking (aka pie in the sky dream) and are there any plans of some 'shared lib' between all the components? For instance, I once started a subprocess wrapper that never got used in that project. Would be nice to re-use standard things like that.

micahjohnson150 commented 4 years ago

I am in favor! This would help me with any upgrades coming to basin_setup in the future.