search

NREL / OpenStudio

OpenStudio is a cross-platform collection of software tools to support whole building energy modeling using EnergyPlus and advanced daylight analysis using Radiance.
https://www.openstudio.net/
Other
486 stars 186 forks source link

4847 logger singleton 3.0 #5110

Closed kbenne closed 4 months ago

kbenne commented 4 months ago

@jmarrec I think this should address the core issue that the logger is not global across the script engines. I think there remains a little bit of work.

  1. I'm going to rename LoggerSingleton since it is not really a singleton (Logger is) and the name is confusing. Also, LoggerSingleton is not part of the public API so I think we can change it without causing harm.
  2. I think we might be able to remove the define SHARED_OS_LIBS. What do you think?
  3. Now that I believe we have the global issue worked out, do you have availability to go through the CLI's use of the logger and undo any workarounds that might be in place? I believe there are some if Windows conditions that attempted to work around the issue.
jmarrec commented 4 months ago

@kbenne can you do a quick walkthrough with some comments please?

jmarrec commented 4 months ago

I rebased onto develop after marking it "Ready for CI" so that CI tries to build it. Curious to see if the windows logger test is fixed

jmarrec commented 4 months ago

Windows failing logger test in "classic" CLI

1/1 Test #3731: OpenStudioCLI.Classic.test_logger_rb ...***Failed    0.87 sec
Namespace(classic=True, logger_file=WindowsPath('D:/git/OpenStudio/src/cli/test/logger_test.rb'), os_cli_path=WindowsPath('D:/git/OpenStudio/build/Products/openstudio.exe'))
Running: D:\git\OpenStudio\build\Products\openstudio.exe classic execute_ruby_script D:\git\OpenStudio\src\cli\test\logger_test.rb
0 LOGGER - STDOUT Warn
1 LOGGER - STDOUT Error
Traceback (most recent call last):
  File "D:/git/OpenStudio/src/cli/test/run_test_logger.py", line 50, in <module>
    raise IOError(f"Expected 3 lines, got {n}")
OSError: Expected 3 lines, got 2

https://github.com/NREL/OpenStudio/blob/800d735ae010994a9159c7e2fde8f72debb46e69/src/cli/test/logger_test.rb#L1-L15

So that OpenStudio::logFree(OpenStudio::Error, "test", "Error") is not printed to the stdout

https://github.com/NREL/OpenStudio/blob/800d735ae010994a9159c7e2fde8f72debb46e69/src/cli/main.cpp#L83-L90

ci-commercialbuildings commented 4 months ago

CI Results for c162c0492a6ba4ec061bfccd159d85286aedb450:

jmarrec commented 4 months ago

Supperseded by #5119