Skip to content
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.

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

Add checks to determine whether "create" and "modify" can safely run #11

Merged
merged 3 commits into from
Sep 29, 2023

Conversation

aholmes
Copy link
Member

@aholmes aholmes commented Sep 27, 2023

Addressing this feedback from @nwiltsie.

This change adds checks to determine whether a scaffolded application already exists based on what directory the scaffolder is running in, and what mode it is running in.


The templated project structure has two layers with <name>:

.
└── nicktest
    ├── build
    │   ├── bdist.macosx-13-arm64
    │   └── lib
    │       └── nicktest
    │           └── endpoints
    ├── nicktest
    │   ├── __pycache__
    │   └── endpoints
    │       └── __pycache__
    └── nicktest.egg-info

You have to be in the parent folder to run python nicktest and launch the server, but you have to be above the parent folder to run bl-python-scaffold correctly; otherwise you get even more layers of nesting:

nicktest $ bl-python-scaffold modify -n nicktest -e goodbye
[INFO   ] Scaffolding application named "nicktest" under directory `/Users/nwiltsie/spike/blapptest/nicktest/nicktest`.
[INFO   ] Running modify mode.
[INFO   ] "goodbye" will be accessible at http://127.0.0.1:5000/goodbye
[INFO   ] Done.
nicktest $ cd ..
blapptest $ tree -I build -I __pycache__ -I *.egg-info
.
└── nicktest
    ├── LICENSE.md
    ├── README.md
    ├── config.toml
    ├── nicktest
    │   ├── __init__.py
    │   ├── __main__.py
    │   ├── endpoints
    │   │   ├── __init__.py
    │   │   ├── application.py
    │   │   ├── helloworld.py
    │   │   └── nicktest.py
    │   └── nicktest
    │       └── endpoints
    │           └── goodbye.py
    └── pyproject.toml

A sanity-check for bl-python-scaffold modify that an endpoints/ directory already exists would be helpful to prevent this. Potentially naming the project directory <name>_project or something could also reduce confusion.

@nwiltsie
Copy link
Member

It took me an embarrassingly long time to actually update the module, but the behavior is exactly what I would expect:

blapptest $ bl-python-scaffold create -n nicktest2
Scaffolding application named "nicktest2" under directory `/Users/nwiltsie/spike/blapptest/nicktest2`.
[INFO   ] Running create mode.
[INFO   ] Creating new application "nicktest2" from basic template ...
[INFO   ] "nicktest2" will be accessible at http://127.0.0.1:5000/nicktest2
Done.
blapptest $ cd nicktest2/
nicktest2 $ bl-python-scaffold modify -n nicktest2 -e subadd
Scaffolding application named "nicktest2" under directory `/Users/nwiltsie/spike/blapptest/nicktest2/nicktest2`.
[INFO   ] Running modify mode.
[CRITICAL] Attempted to modify an existing application from a directory that is not the existing application's parent directory. This is not supported. Change your working directory to the application's parent directory.
Done.

# is less than ideal and potentionally dangerous. we should
# consider forking here or doing something else to prevent
# the global modification.
sys.path.insert(0, str(application_import_path))
Copy link
Member

Choose a reason for hiding this comment

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

I know of the existence of importlib and a quick Googlin' turned this up - I make no promises that this is actually addressing this problem: https://docs.python.org/3/library/importlib.html#importing-a-source-file-directly

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately it's not the source file itself I need here - it's the package/module. Otherwise, SourceFileLoader.load_module would work and reduces the complexity of using spec_from_file_location directly (and which I'm using in the module hook lookup, which I should replace).

The package/module is needed because _version.py references the module's metadata with importlib.metadata to get the software version from pyproject.toml. Just loading the source file fails because a source file does not contain package metadata regardless of its location. So we have to fake that the package is installed in order for Python to treat the source file as if it's loaded from a package.

Copy link
Member

Choose a reason for hiding this comment

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

Alas! It seems my dashed-off suggestion is not the solution to all of your problems.

@aholmes aholmes merged commit a757cae into main Sep 29, 2023
1 check failed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants