-
Notifications
You must be signed in to change notification settings - Fork 66
v3.17.5 /simulated_load POST request updates #694
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
Conversation
…tion Optional load_type (default=electric) Optional year with doe_reference_name
/simulated_load POST request updates
Also some type ensurances
The beauty of the POST is that we can effectively pass the .json straight to the function as a dictionary with minimal data processing. This preserves all intentionally inputs validation/checking that was there before.
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.
Pull request overview
This PR updates the /simulated_load endpoint to improve POST request handling by making load_type and year optional inputs where appropriate, fixing latitude/longitude pass-through for POST requests with doe_reference_name, and simplifying the input handling by directly parsing the JSON body into the inputs dictionary.
Key changes:
- Makes
load_typeoptional for both GET and POST requests with default value of "electric" - Makes
yearoptional for POST requests withdoe_reference_name(consistent with GET behavior) - Simplifies POST request handling by directly parsing JSON body into inputs dictionary instead of manually mapping each field
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| reoptjl/views.py | Updated GET request to conditionally add load_type if present; refactored POST request handling to directly parse JSON body and simplified validation logic for required fields |
| reoptjl/test/test_http_endpoints.py | Added comprehensive test suite with 8 test cases covering various POST request scenarios including validation, normalization, reference names, and hybrid buildings |
| CHANGELOG.md | Documents the changes made in v3.17.5 including the optional parameters and lat/long pass-through fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…GET to POST due to potential payload size Even though the GET method works, for large payloads in particular, this should be a POST.
reoptjl/views.py
Outdated
| elif request.GET.get(key) is not None: | ||
| inputs[key] = float(request.GET.get(key)) | ||
|
|
||
| if request.method == "POST": |
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.
is any other method accepted for this endpoint? If not, could include an else to this statement with something like "return JsonResponse({"Error": "Only POST method is supported for this endpoint"}, status=405)" (which we do for get_load_metrics()
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.
No, good idea. Added in e819d274479cc5f365c997a2c32916c05015e3d6
adfarth
left a comment
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.
Just one comment to address but I approve pending a response to that! Also curious to confirm there was web tool testing after the second and third commit mentioned in the PR description?
Thanks! I'll address that one. Yes I tested the webtool after making the last commit which fixed some missing params being sent to Julia, and it made Erika have to fix the data type for lat/long which were being sent as a string, so had to convert to a number. |
PR:
Plus this commit for fixes found during web dev testing:
And this commit after realizing I missed another optional input for cooling load and that we don't have to map/handle each parameter of the POST request body to the inputs dictionary - we can just parse and pass the .json body directly to the inputs dictionary (which is the convenience of this POST structure for this endpoint):
Finally (final final) last commit directly made to develop: update the way the API internally calls the Julia API for the /simulated_load endpoint to be a POST for best-practice with potentially larger payloads (e.g. with a load_profile)