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
494 stars 188 forks source link

Add `json_schemer` gem #4969

Open shorowit opened 12 months ago

shorowit commented 12 months ago

Enhancement Request

OpenStudio includes the json-schema gem, but it is barely maintained and many users recommend switching to json_schemer gem. Not only is the json_schemer gem well maintained, but it supports newer drafts of the JSON schema that are heavily utilized these days. The json-schema gem has no current progress towards supporting newer drafts of the schema.

For DOE's Home Energy Score project, we are unable to use the current json-schema gem because it doesn't support JSON Schema draft-07. This means that we can't validate JSON files submitted and has occasionally led to issues.

(Perhaps OpenStudio should consider replacing the json-schema gem, but that would be a breaking change.)

jmarrec commented 12 months ago

No problem. This is a pure ruby gem, no native ext, so we shouldn't need any patching because MSVC doesn't support C99 etc.

@wenyikuang Do you want to take this one?

jmarrec commented 12 months ago

Added the PR at https://github.com/NREL/openstudio-gems/pull/64.

Assuming it works, packages should be uploaded to S3 and OpenStudio's CMakeLists.txt should be updated to match.

(I'll have to build the mac M1 one myself and have @wenyikuang upload it to s3 as well, unless the arm64 mac CI machines are ready. Edit: built and hosted on google drive, link on slack)

wenyikuang commented 12 months ago

No problem. This is a pure ruby gem, no native ext, so we shouldn't need any patching because MSVC doesn't support C99 etc.

@wenyikuang Do you want to take this one?

Sure, let me take a look.

jmarrec commented 12 months ago

@shorowit reported this error, that I can reproduce on Ubuntu 20.04

$os_build_rel3/Products/openstudio -e "require 'json_schemer'"
Error executing argv: ["-e", "require 'json_schemer'"]
Error: :/ruby/2.7.0/gems/json_schemer-2.0.0/lib/json_schemer/format/hostname.rb:36: invalid character property name {Hiragana}: /[\p{Hiragana}\p{Katakana}\p{Han}]/ in eval:193:in `eval'

The test is actually failing, even though Ctest / minitest says it's fine.

3828:   1) Error:
3828: EmbeddedRuby_Test#test_json_schemer:
3828: SyntaxError: :/ruby/2.7.0/gems/json_schemer-2.0.0/lib/json_schemer/format/hostname.rb:36: invalid character property name {Hiragana}: /[\p{Hiragana}\p{Katakana}\p{Han}]/
3828:     eval:193:in `eval'
3828:     eval:193:in `require_embedded_absolute'
3828:     eval:178:in `block in require_embedded'
3828:     eval:172:in `each'
3828:     eval:172:in `require_embedded'
3828:     eval:147:in `rescue in require'
3828:     eval:141:in `require'
3828:     :/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in `require'
3828:     :/ruby/2.7.0/gems/json_schemer-2.0.0/lib/json_schemer.rb:18:in `<main>'
3828:     eval:193:in `eval'
3828:     eval:193:in `require_embedded_absolute'
3828:     eval:178:in `block in require_embedded'
3828:     eval:172:in `each'
3828:     eval:172:in `require_embedded'
3828:     eval:147:in `rescue in require'
3828:     eval:141:in `require'
3828:     :/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in `require'
3828:     /media/DataExt4/Software/Others/OpenStudio3/src/cli/test/test_embedded_ruby.rb:237:in `test_json_schemer'
3828: 
3828: 1 runs, 0 assertions, 0 failures, 1 errors, 0 skips
1/1 Test #3828: CLITest-test_embedded_ruby-json_schemer ...   Passed   10.62 sec

The following tests passed:
    CLITest-test_embedded_ruby-json_schemer

100% tests passed, 0 tests failed out of 1
jmarrec commented 12 months ago

This works: openstudio -e "/[\p{Alnum}]/", this doesn't openstudio -e "/[\p{Han}]/"

jmarrec commented 11 months ago

https://github.com/davishmcclurg/json_schemer/blob/6c88530c594a953a11061c6abf642e451b1ad522/lib/json_schemer/format/hostname.rb#L36

jmarrec commented 11 months ago

I realize that the conan-openstudio-ruby test_package works fine. Which made me realize this is a regression that dates to when we added python measures and made the CLI non statically linked to ruby

$ /usr/local/openstudio-3.4.0/bin/openstudio -e "puts /[\p{Han}]/"
(?-mix:[\p{Han}])
$ /usr/local/openstudio-3.5.0/bin/openstudio -e "puts /[\p{Han}]/"
Error executing argv: ["-e", "puts /[\\p{Han}]/"]
Error: eval:48: invalid character property name {Han}: /[\p{Han}]/ in :/openstudio_cli.rb:777:in `eval'
:/openstudio_cli.rb:777:in `block in execute'
:/openstudio_cli.rb:775:in `each'
:/openstudio_cli.rb:775:in `execute'
:/openstudio_cli.rb:1973:in `<main>'
eval:188:in `eval'
eval:188:in `require_embedded_absolute'
eval:173:in `block in require_embedded'
eval:167:in `each'
eval:167:in `require_embedded'
eval:126:in `require'
eval:3:in `<main>'
jmarrec commented 7 months ago

With ruby3 I'm having an issue due to this gem on windows.

It depends on simpleidn, which depends on unf_ext, which is a native C++ extension (and /std:c++20 at that!).

https://github.com/knu/ruby-unf_ext/blob/c72a36d0a5ea9fe3950611b0f289fc68a2595fcf/ext/unf_ext/extconf.rb#L8-L13

And it bakes some directives into the unf_ext.lib such as /FAILIFMISMATCH:RuntimeLibrary=MD_DynamicRelease. Meaning I can't link to it when I build OS SDK as Debug. Cause of https://github.com/NREL/openstudio-gems/issues/72

https://github.com/davishmcclurg/json_schemer/commit/8e358d52afbdeae6c0091965167b4702f0c5b60a

jmarrec commented 7 months ago

No problem. This is a pure ruby gem, no native ext, so we shouldn't need any patching because MSVC doesn't support C99 etc.

That couldn't have been more wrong :)

DavidGoldwasser commented 1 week ago

@jmarrec what do you think about this in 3.9? Is there a path to do that? Could a Python measure in the workflow do this instead of Ruby. Maybe OS add a JSON validation Python library to our Python

shorowit commented 1 week ago

If we went the python route, which would probably work fine for our use case, I think the library we'd want added to OS is jsonschema.