Skip to content

First pass at teasing apart generateconfs into separate functions. #82

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

First pass at teasing apart generateconfs into separate functions. #82

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9955216
First pass at teasing apart generateconfs into separate functions.
albu-diku Jul 13, 2024
c78bd72
fixup
albu-diku Jul 15, 2024
fb4857a
fixup
albu-diku Jul 15, 2024
9400618
fixup
albu-diku Jul 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions mig/install/generateconfs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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__), "../..")))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


# NOTE: moved mig imports into try/except to avoid autopep8 moving to top!
try:
Expand Down Expand Up @@ -355,9 +355,9 @@ def usage(options):
# Remove default values to use generate_confs default values
if val == 'DEFAULT':
del settings[key]
conf = generate_confs(**settings)
options = generate_confs(**settings)
# TODO: avoid reconstructing this path (also done inside generate_confs)
instructions_path = "%s/instructions.txt" % conf['destination_path']
instructions_path = os.path.join(options['destination_dir'], 'instructions.txt')
try:
instructions_fd = open(instructions_path, "r")
instructions = instructions_fd.read()
Expand Down
Loading
Loading