Measure manager fixups and improvements #5304

jmarrec · 2024-11-14T12:47:01Z

Pull request overview

Prompted by OpenStudio-PAT about to adopt the C++ CLI, made a few last minute changes
- New C++ MeasureManager OpenStudio-PAT#279
  - Replaced with New mm reboot OpenStudio-PAT#284
Renabled testing and comparison of Ruby versus C++ CLI
- Modified the key in /internal_state to match ruby
Added some kind of feedback (logs) for the server itself, so in the terminal that runs it you see some kind of output showing the connections it receives, like WEBrick would do

color()(set -o pipefail;"$@" 2> >(sed $'s,.*,\e[31m&\e[m,'>&2))
export -f color
color $os_build3/Products/openstudio measure -s 1065

Pull Request Author

Model API Changes / Additions
Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate)
Model API methods are tested (in src/model/test)
EnergyPlus ForwardTranslator Tests (in src/energyplus/Test)
If a new object or method, added a test in NREL/OpenStudio-resources: Add Link
If needed, added VersionTranslation rules for the objects (src/osversion/VersionTranslator.cpp)
Verified that C# bindings built fine on Windows, partial classes used as needed, etc.
All new and existing tests passes
If methods have been deprecated, update rest of code to use the new methods

Labels:

If change to an IDD file, add the label IDDChange
If breaking existing API, add the label APIChange
If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

This will not be exhaustively relevant to every PR.

Perform a Code Review on GitHub
Code Style, strip trailing whitespace, etc.
All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
Labeling is ok
If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
If feature, test running new feature, try creative ways to break it
CI status: all green or justified

jmarrec · 2024-11-14T12:47:46Z

python/SetupPython.cmake

+execute_process(COMMAND ${Python_EXECUTABLE} -c "import requests; print(requests.__version__)"
+  RESULT_VARIABLE _PyRequests_STATUS
+      OUTPUT_VARIABLE PyRequests_Version
+      ERROR_QUIET
+      OUTPUT_STRIP_TRAILING_WHITESPACE
+)
+if(_PyRequests_STATUS AND NOT _PyRequests_STATUS EQUAL 0)
+  message(AUTHOR_WARNING "requests isn't installed on your system python, so some tests won't be run. Run `pip install requests`")
+  set(PyRequests_AVAILABLE OFF)
+else()
+  message("Found Python requests: ${PyRequests_Version}")
+  set(PyRequests_AVAILABLE ON)
+endif()


Detect if requests is installed on the system python

jmarrec · 2024-11-14T12:48:05Z

src/cli/CMakeLists.txt

+    if (PyRequests_AVAILABLE)
+      add_test(NAME OpenStudioCLI.test_measure_manager
+        COMMAND ${Python_EXECUTABLE} -m pytest --verbose --os-cli-path $<TARGET_FILE:openstudio> "${CMAKE_CURRENT_SOURCE_DIR}/test/test_measure_manager.py"
+        WORKING_DIRECTORY "${PROJECT_BINARY_DIR}/Testing/"
+      )
+      add_test(NAME OpenStudioCLI.Classic.test_measure_manager
+        COMMAND ${Python_EXECUTABLE} -m pytest --verbose --use-classic --os-cli-path $<TARGET_FILE:openstudio> "${CMAKE_CURRENT_SOURCE_DIR}/test/test_measure_manager.py"
+        WORKING_DIRECTORY "${PROJECT_BINARY_DIR}/Testing/"
+      )
+    endif()


Register the Classic (Ruby) and C++ version of the test_measure_manager.py

jmarrec · 2024-11-14T12:48:14Z

src/cli/test/conftest.py

 def pytest_addoption(parser):
    parser.addoption("--os-cli-path", type=validate_file, help="Path to the OS CLI")  # , required=True
-
+    parser.addoption("--use-classic", action="store_true", help="Force use the Classic CLI")


need addopt

jmarrec · 2024-11-14T12:48:24Z

src/cli/test/conftest.py

+
+@pytest.fixture(scope="module")
+def use_classic_cli(request):
+    use_classic = request.config.getoption("--use-classic")
+    return use_classic


Set it as a fixture for use

jmarrec · 2024-11-14T12:48:48Z

src/cli/test/test_measure_manager.py

+}
+
+BASE_INTERNAL_STATE_LABS: Dict[str, Any] = {
+    "idfs": [],


This is the only key that the C++ CLI added in /internal_state

jmarrec · 2024-11-14T12:53:43Z

src/cli/MeasureManager.cpp

  }

  auto& measureInfos = result["measure_info"];
+  measureInfos = Json::arrayValue;


jmarrec · 2024-11-14T12:54:02Z

src/cli/MeasureManager.cpp

    return;
  }

-  message.reply(web::http::status_codes::BadRequest, toWebJSON(fmt::format("Error, unknown path '{}'", uri)));


In Get, was returning a 400 instead of a 404 when unknown endpoint

jmarrec · 2024-11-14T12:54:18Z

src/cli/MeasureManager.cpp

+void MeasureManagerServer::unknown_endpoint(web::http::http_request& message) {
+  const std::string uri = toString(web::http::uri::decode(message.relative_uri().path()));
+  message.reply(web::http::status_codes::BadRequest, toWebJSON(fmt::format("Error, unknown path '{}'", uri)));
+  print_feedback(message, web::http::status_codes::NotFound);
+}


use a function for both GET and POST when 404

jmarrec · 2024-11-14T12:54:33Z

src/cli/MeasureManager.cpp

+void MeasureManagerServer::print_feedback(const web::http::http_request& message, web::http::status_code status_code) {
+  const std::string uri = toString(web::http::uri::decode(message.relative_uri().path()));
+  const std::string method = toString(message.method());
+  const std::string http_version = message.http_version().to_utf8string();
+  const std::string timestamp = openstudio::DateTime::now().toXsdDateTime();
+  fmt::print(status_code == web::http::status_codes::OK ? stdout : stderr, "[{}] \"{} {} {}\" {}\n", openstudio::DateTime::now().toXsdDateTime(),
+             method, uri, http_version, status_code);
+}


jmarrec · 2024-11-14T12:56:09Z

ruby/engine/measure_manager_server.rb

        result = {}

-        data = JSON.parse(request.body, {:symbolize_names=>true})
+        data = JSON.parse(request.body || "{}", {:symbolize_names=>true})


Minor tweaks in ruby CLI. When body is empty, you don't want to get bad error on JSON.parse with an ugly backtrace and No implicit conversion of nil into String isn't a good error message

jmarrec · 2025-03-11T08:32:44Z

@DavidGoldwasser @kbenne this has been opened since November, can someone take a look please? I just rebased onto develop

… with classic cli and find differences

…running, kinda like Webbrick was doing success, on stdout: [2024-11-14T10:21:46+01:00] "POST /reset HTTP/1.1" 200 failure, on stderr: [2024-11-14T10:22:09+01:00] "GET /dsd HTTP/1.1" 400

…e on JSON.parse No implicit conversion of nil into String isn't a good error message

ci-commercialbuildings · 2025-05-06T07:17:13Z

CI Results for dfc7305:

Ubuntu 24.04:
- OpenStudio-3.10.0-alpha+dfc73053e8-Ubuntu-24.04-x86_64.deb
- OpenStudio-3.10.0-alpha+dfc73053e8-Ubuntu-24.04-x86_64.tar.gz
Ubuntu 22.04:
- OpenStudio-3.10.0-alpha+dfc73053e8-Ubuntu-22.04-x86_64.deb
- OpenStudio-3.10.0-alpha+dfc73053e8-Ubuntu-22.04-x86_64.tar.gz
Darwin:
- OpenStudio-3.10.0-alpha+dfc73053e8-Darwin-x86_64.dmg
- OpenStudio-3.10.0-alpha+dfc73053e8-Darwin-x86_64.tar.gz
Windows:
- OpenStudio-3.10.0-alpha+dfc73053e8-Windows.exe
- OpenStudio-3.10.0-alpha+dfc73053e8-Windows.tar.gz
Code Coverage:

jmarrec · 2025-05-12T09:22:45Z

@kbenne can you take a quick look here please? I'd like to merge it in.

jmarrec commented Nov 14, 2024

View reviewed changes

jmarrec added component - CLI component - Measure Manager Pull Request - Ready for CI This pull request if finalized and is ready for continuous integration verification prior to merge. labels Nov 14, 2024

jmarrec self-assigned this Nov 14, 2024

DavidGoldwasser added this to the OpenStudio SDK 3.10.0 milestone Nov 14, 2024

jmarrec requested a review from kbenne December 19, 2024 15:06

jmarrec force-pushed the measure_manager_fixups_and_improvements branch from c4fb845 to 417a930 Compare December 19, 2024 15:06

jmarrec force-pushed the measure_manager_fixups_and_improvements branch from 417a930 to 27df95b Compare January 7, 2025 16:58

jmarrec force-pushed the measure_manager_fixups_and_improvements branch from 27df95b to bc47e96 Compare March 11, 2025 08:32

jmarrec mentioned this pull request Mar 20, 2025

#4830 - Add a new ReportingMeasure::modelOutputRequests(model, runner, argument_map) that runs before E+ FT #5367

Merged

19 tasks

jmarrec force-pushed the measure_manager_fixups_and_improvements branch from bc47e96 to cde06b3 Compare March 20, 2025 10:07

jmarrec added 6 commits May 6, 2025 08:55

Add the test_measure_manager.py to the Ctest, revamp it, make it work…

16ee7a7

… with classic cli and find differences

Align C++ CLI internal_state with Ruby one

260ee60

Add a helper to set 404 error (the GET was returning a 400 ...)

2faa309

Move "/" to a member function like the rest

310865f

Add feedback (logs) to the console where the MeasureManagerServer is …

76bd106

…running, kinda like Webbrick was doing success, on stdout: [2024-11-14T10:21:46+01:00] "POST /reset HTTP/1.1" 200 failure, on stderr: [2024-11-14T10:22:09+01:00] "GET /dsd HTTP/1.1" 400

Slight tweak to Ruby measure_manager: if POST body is nil, don't chok…

dfc7305

…e on JSON.parse No implicit conversion of nil into String isn't a good error message

jmarrec force-pushed the measure_manager_fixups_and_improvements branch from cde06b3 to dfc7305 Compare May 6, 2025 06:55

jmarrec merged commit b9ed08b into develop May 14, 2025
2 of 6 checks passed

jmarrec deleted the measure_manager_fixups_and_improvements branch May 14, 2025 13:12

Measure manager fixups and improvements #5304

Measure manager fixups and improvements #5304

Uh oh!

Conversation

jmarrec commented Nov 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request overview

Pull Request Author

Review Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmarrec commented Mar 11, 2025

Uh oh!

ci-commercialbuildings commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jmarrec commented May 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jmarrec commented Nov 14, 2024 •

edited

Loading

ci-commercialbuildings commented May 6, 2025 •

edited

Loading

jmarrec commented May 12, 2025 •

edited

Loading