await_spi_available added, add retry to get_region, and repackage logger #31
Conversation
marvinmarnold
force-pushed
the
marvinmarnold/fix-logger-packaging
branch
from
814b060 to
cf1910a
Compare
marvinmarnold
changed the title
marvinmarnold
force-pushed
the
marvinmarnold/fix-logger-packaging
branch
from
cf1910a to
001bdf5
Compare
|
@NebraLtd/developers this should be re-reviewed. I've added net new logic. |
marvinmarnold
force-pushed
the
marvinmarnold/fix-logger-packaging
branch
3 times, most recently
from
97ac2c6 to
974cad6
Compare
|
|
||
| logger.debug("No REGION_OVERRIDE defined, will retrieve from miner.") | ||
| with open(REGION_FILEPATH) as region_file: | ||
| LOGGER.debug("No region override set (value = %s), will retrieve from miner." % region_override) # noqa: E501 |
There was a problem hiding this comment.
Is the idea here that we can pass other values to the retry_get_region command for different services should we need to?
Rather than hard coding it with the REGION_OVERRIDE and REGION_FILEPATH env variables?
There was a problem hiding this comment.
Portability to other services is one reason. The bigger reason is I think its hard to maintain services with environment variables that are read in the depths of the code. I prefer to parameterize everything and make it up to the caller to decide where those parameters come from. Both in hm-config and hm-pktfwd refactors, __main__.py acts as a single entry point where the environment is read (eg: https://github.com/NebraLtd/hm-pktfwd/pull/59/files#diff-a6ac805adb62e68e9273e1c2fc120902571d800d5596a1df4dcb316b303d520aR30).
So if we decided to read all these values from a .env file in the future instead of the actual environment, we can just change the main function without having to dig through the code to find all the places the environment is read.
There was a problem hiding this comment.
Makes sense. Looks like we don't actually use retry_get_region anywhere yet in any case?
There was a problem hiding this comment.
Only in the upcoming refactor of hm-pktfwd.
marvinmarnold
force-pushed
the
marvinmarnold/fix-logger-packaging
branch
from
9d7c921 to
7000d7b
Compare
- repackage logger from hm_pyhelper.utils to hm_pyhelper.logger - LOGGER should be instantiated as uppercase by convention - add tests - add retry logic to get_region - create await_spi_available helper - fix override logic
marvinmarnold
force-pushed
the
marvinmarnold/fix-logger-packaging
branch
from
83fc98d to
fbc8817
Compare
This was previously in a utils module that was not being used for other
tools. It was also missing an init.py and was not being properly
exported. This pattern is slightly less verbose.