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

JsonPrinter and SBEIR Schema codegen issues #560

Closed
aksharp opened this issue Jun 7, 2018 · 6 comments
Closed

JsonPrinter and SBEIR Schema codegen issues #560

aksharp opened this issue Jun 7, 2018 · 6 comments

Comments

@aksharp
Copy link

aksharp commented Jun 7, 2018

Ran into few issues while evaluating SBE 1.8.1

  1. Issues with JsonPrinter (would like to use it for logging/toolchain as it is faster than alternatives)
  • Prints invalid JSON
  • Drops enum and bool fields
  • Rounds up numbers
  1. IR Schema file
  • Enum values are missing when using sbeir schema with SBE Tool to codegen

POC code is available on github here: https://github.com/aksharp/sbe-poc

Failing test
https://github.com/aksharp/sbe-poc/blob/master/src/test/scala/sbe/JsonComparison.scala

mjpt777 added a commit that referenced this issue Jun 17, 2018
… Full enums are serialised and fields retain the constant property and encoded length rather than the type. Issue #560.
mjpt777 added a commit that referenced this issue Jun 17, 2018
…Full enums are serialised and fields retain the constant property and encoded length rather than the type. Issue #560.
@mjpt777
Copy link
Contributor

mjpt777 commented Jun 17, 2018

Thanks @aksharp for the report. There was indeed an issue with encoded IR which I've now fixed.

The JsonPrinter was introduced for debugging purposes and is incomplete. If I get time I'll look at fleshing out the remainder of the functionality.

@mjpt777
Copy link
Contributor

mjpt777 commented Jun 17, 2018

@billsegall Could you have a look at modifying the Rust codec so this test passes again and see if the C# needs any attention?

https://github.com/real-logic/simple-binary-encoding/blob/master/sbe-tool/src/test/java/uk/co/real_logic/sbe/generation/rust/RustGeneratorTest.java#L239

@mjpt777
Copy link
Contributor

mjpt777 commented Jun 17, 2018

I've now fleshed out the features to support all types.

@mjpt777
Copy link
Contributor

mjpt777 commented Jun 18, 2018

@ZackPierce Can you have a look at the Rust changes? I should be fairly simple.

@ZackPierce
Copy link
Contributor

Sure, I'll look into it this week.

@ZackPierce
Copy link
Contributor

Yep, I think I've got it sorted out. Will toss up a PR shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants