-
Notifications
You must be signed in to change notification settings - Fork 22
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
refactor: make api available outside flask context #130
Conversation
@@ -26,11 +26,8 @@ def get_script_path(): | |||
return str(path_to_script) | |||
|
|||
|
|||
@app.route("/launch/<int:scenario_id>", methods=["POST"]) | |||
def launch_simulation(scenario_id): | |||
def launch_simulation(scenario_id, threads=None, solver=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're going to be using this to launch simulations from powersimdata
, should we add a check for available solvers here just so that it fails faster? i.e. from pyreisejl.utility.launchers import get_available_solvers
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We currently have this validation in powersimdata, not sure if we'd want to move it here at some point but that gets into a bit of a design decision so thinking we can save that for later.
Looks good! I'm assuming to the |
Yep, exactly! |
EXECUTE_LIST = os.path.join(DATA_ROOT_DIR, "ExecuteList.csv") | ||
EXECUTE_DIR = os.path.join(DATA_ROOT_DIR, "tmp") | ||
INPUT_DIR = os.path.join(DATA_ROOT_DIR, "data", "input") | ||
OUTPUT_DIR = os.path.join(DATA_ROOT_DIR, "data", "output") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One day, it will be nice to find a ways to not set these variables when we use PowerSimData to launch REISE.jl. What is particularly ugly is that the path needs to be identical to the ones set in PowerSimData
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
Purpose
Enable these functions to be called outside of flask, so a user can launch a simulation and check the status without running the web server, which is mainly useful for communication between containers.
What it does
Separate the functions from flask (no route decorator or usage of
jsonify
, which creates an http response object).By adding the path to REISE.jl to
sys.path
, we can import this module within powersimdata.Testing
Ongoing, along with the Breakthrough-Energy/PowerSimData#477
Time to review
5 mins