-
Notifications
You must be signed in to change notification settings - Fork 41
Description
I was going to start working on the implementation of the server.py test code, but there were some areas where I thought the process could be improved, so I made this issue!
The current implementation of the Flask endpoint in app.py uses a subprocess to execute the run function from cli.py. I think this approach introduces extra overhead and complexity.
Current Code
@app.route("/")
def default_route():
... snip ...
@app.route("/")
def default_route():
query_parameters = urllib.parse.parse_qsl(
request.query_string.decode(), keep_blank_values=True
)
parsed_parameters = []
for key, value in query_parameters:
if value:
parsed_parameters.append(f"{key}={value}")
else:
parsed_parameters.append(key)
args = ",".join(parsed_parameters)
async def run_subprocess():
try:
result = subprocess.run(
["python3", "src/cli.py", args],
capture_output=True,
text=True,
check=True,
)
return result.stdout
except subprocess.CalledProcessError as e:
print("Error message from subprocess:", e.stderr)
raise e
loop = asyncio.new_event_loop()
asyncio.set_event_loop(loop)
result = loop.run_until_complete(run_subprocess())
return resultSuggestion
How about refactoring the code to import and call the run function directly from cli.py and eliminate the need for sub-processes?
An image of the proposed amendment is below. (although error handling, etc. will need to be considered separately)
from src.cli import run # Direct import
@app.route("/")
def default_route():
query_parameters = urllib.parse.parse_qsl(
request.query_string.decode(), keep_blank_values=True
)
parsed_parameters = []
for key, value in query_parameters:
if value:
parsed_parameters.append(f"{key}={value}")
else:
parsed_parameters.append(key)
args = ",".join(parsed_parameters)
result = run(args) # Direct function call
return resultOf course, the cli.py side must also be modified to make this modification.
I think the advantages of calling cli.py directly are as follows.
-
Reduces overhead and complexity
- Using subprocess adds extra layers of process creation and inter-process communication. Directly calling the function within the same process simplifies the code and avoids these unnecessary steps.
-
Simplifies error handling
- When using sub-processes,
cli.pyis executed by the new process, which makes it difficult to catch errors when they occur.cli.pycan be called directly to easily locate the error (easily tracked by the IDE's debugger, etc.) (which can be easily tracked using the IDE's debugger, etc.).
- When using sub-processes,
@ryansurf
However, I am wrong and may have missed something. If you are using Subprocess by design, please tell me why!