First pass at teasing apart generateconfs into separate functions. #82
Conversation
albu-diku
force-pushed
the
refactor/split-up-config-generation
branch
from
ab98e34 to
6680e39
Compare
Previous work lead to the observation that in the many LoC that make up generateconfs there are in fact distinct phases: the creation of a finalised values that are to be written and the injection of those values into various templates (and the output of instructions). The earlier work also established coverage in the form of a test of the generated files and, with this in-hand, it became feasible to refactor this safely and without fear of breakge. Opt to do this. Apply the notion of distinct phases to the code itself: break generateconfs() into a series of distinct functions. Be minimal in doing thus leaving a large amount of potential for e.g. improved naming and further alignment on the table - but the result is a rather compact diff that nevertheless makes makes it possible to get at the data structures flowing through the process thus enablng further work. Note that this exercise causes path handling to begin to consolidate which allows distiguishing configuration values from the options that govern where outputs will be written.
albu-diku
force-pushed
the
refactor/split-up-config-generation
branch
from
7f6f448 to
9400618
Compare
| @@ -42,7 +42,7 @@ | |||
|
|
|||
| # NOTE: __file__ is /MIG_BASE/mig/install/generateconfs.py and we need MIG_BASE | |||
|
|
|||
| sys.path.append(os.path.dirname(os.path.dirname(os.path.dirname(__file__)))) | |||
| sys.path.append(os.path.realpath(os.path.join(os.path.dirname(__file__), "../.."))) | |||
There was a problem hiding this comment.
I'd would prefer to avoid this construct if it's really a matter of handling __file__ possibly being a relative path. In that case I'd much prefer to call abspath (or realpath) on file first thing. It's only if we e.g. cross symlinks that realpath should be needed...
sys.path.append(os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))))
My brief tests indicate that it will work no matter how __file__ is executed and across python versions.
There was a problem hiding this comment.
Ok - for the record this was changed because I was unable to directly execute the script with the previous formulation.
That said, really this dovetails with PYTHONPATH and once that is honoured everywhere these all go away. That't not to say it shouldn't be right in-tree, but for the sake of record this is understood as being legacy anyway and there will be a mass removal of straggling stuff like this once it is safe to do so.
There was a problem hiding this comment.
alright, and yes that sounds like PYTHONPATH and/or abspath might be missing in certain variations of the expansion. If we can just add the abspath(__file__) in those situations I'm quite confident we won't introduce other side effects until we can drop the workaround completely.
|
Manually merged through svn with minor modifications. |
Previous work lead to the observation that in the many LoC that make up generateconfs there are in fact distinct phases: the creation of a finalised values that are to be written and the injection of those values into various templates (and the output of instructions).
The earlier work also established coverage in the form of a test of the generated files and, with this in-hand, it became feasible to refactor this safely and without fear of breakge.
Opt to do this. Apply the notion of distinct phases to the code itself: break generateconfs() into a series of distinct functions. Be minimal in doing thus leaving a large amount of potential for e.g. improved naming and further alignment on the table - but the result is a rather compact diff that nevertheless makes makes it possible to get at the data structures flowing through the process thus enablng further work.
Note that this exercise causes path handling to begin to consolidate which allows distiguishing configuration values from options that govern where outputs will be written.