Skip to content
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

Add json_schemer gem #4969

Open
shorowit opened this issue Sep 18, 2023 · 11 comments · Fixed by #4974
Open

Add json_schemer gem #4969

shorowit opened this issue Sep 18, 2023 · 11 comments · Fixed by #4974

Comments

@shorowit
Copy link
Contributor

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

@shorowit shorowit added Enhancement Request Triage Issue needs to be assessed and labeled, further information on reported might be needed labels Sep 18, 2023
@jmarrec
Copy link
Collaborator

jmarrec commented Sep 18, 2023

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
Copy link
Collaborator

jmarrec commented Sep 18, 2023

Added the PR at NREL/openstudio-gems#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
Copy link
Collaborator

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 added a commit that referenced this issue Sep 20, 2023
Fix #4969 - add `json_schemer` to embedded ruby gems
@jmarrec jmarrec reopened this Sep 20, 2023
@jmarrec
Copy link
Collaborator

jmarrec commented Sep 20, 2023

@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
Copy link
Collaborator

jmarrec commented Sep 20, 2023

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

@jmarrec
Copy link
Collaborator

jmarrec commented Sep 26, 2023

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
Copy link
Collaborator

jmarrec commented Feb 14, 2024

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 NREL/openstudio-gems#72

davishmcclurg/json_schemer@8e358d5

@jmarrec
Copy link
Collaborator

jmarrec commented Feb 14, 2024

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

jmarrec added a commit to jmarrec/openstudio-gems that referenced this issue Feb 14, 2024
jmarrec added a commit to jmarrec/openstudio-gems that referenced this issue Feb 14, 2024
@DavidGoldwasser
Copy link
Collaborator

DavidGoldwasser commented Sep 5, 2024

@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

@DavidGoldwasser DavidGoldwasser added this to the OpenStudio SDK 3.9.0 milestone Sep 5, 2024
@shorowit
Copy link
Contributor Author

shorowit commented Sep 5, 2024

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.

@DavidGoldwasser DavidGoldwasser removed the Triage Issue needs to be assessed and labeled, further information on reported might be needed label Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants