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
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

@@ -12,11 +12,16 @@ def validate_file(arg):

 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

  for (const auto& [measureDirPath, bclMeasureInfo] : m_measures) {
    Json::Value mInfo(Json::objectValue);
    measures.append(bclMeasureInfo.measure.toJSON());
  }

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


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

src/cli/MeasureManager.cpp

@@ -609,7 +616,7 @@ void MeasureManagerServer::handle_get(web::http::http_request message) {
    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

@@ -119,7 +119,7 @@ def do_POST (request, response)

        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

… 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-01-07T17:13:24Z

CI Results for 27df95b:

Ubuntu 24.04:
- OpenStudio-3.9.1-alpha+27df95bad9-Ubuntu-24.04-x86_64.deb
- OpenStudio-3.9.1-alpha+27df95bad9-Ubuntu-24.04-x86_64.tar.gz
Ubuntu 22.04:
Darwin:
- OpenStudio-3.9.1-alpha+27df95bad9-Darwin-x86_64.dmg
- OpenStudio-3.9.1-alpha+27df95bad9-Darwin-x86_64.tar.gz
Windows:
- OpenStudio-3.9.1-alpha+27df95bad9-Windows.exe
- OpenStudio-3.9.1-alpha+27df95bad9-Windows.tar.gz
Code Coverage:

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 added 6 commits January 7, 2025 17:58

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

7c11109

… with classic cli and find differences

Align C++ CLI internal_state with Ruby one

93aee74

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

5461856

Move "/" to a member function like the rest

4d76d65

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

d9d427b

…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…

27df95b

…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 417a930 to 27df95b Compare January 7, 2025 16:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Measure manager fixups and improvements #5304

Measure manager fixups and improvements #5304

jmarrec commented Nov 14, 2024 •

edited

Loading

jmarrec Nov 14, 2024

jmarrec Nov 14, 2024

jmarrec Nov 14, 2024

jmarrec Nov 14, 2024

jmarrec Nov 14, 2024

jmarrec Nov 14, 2024

jmarrec Nov 14, 2024

jmarrec Nov 14, 2024

jmarrec Nov 14, 2024

jmarrec Nov 14, 2024

ci-commercialbuildings commented Jan 7, 2025 •

edited

Loading

Measure manager fixups and improvements #5304

Are you sure you want to change the base?

Measure manager fixups and improvements #5304

Conversation

jmarrec commented Nov 14, 2024 • edited Loading

Pull request overview

Pull Request Author

Review Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ci-commercialbuildings commented Jan 7, 2025 • edited Loading

jmarrec commented Nov 14, 2024 •

edited

Loading

ci-commercialbuildings commented Jan 7, 2025 •

edited

Loading