Add SBE Defaults

[ryanohnemus]

I found the project very useful (thank you!) and added the following for my own usage. I thought others might like these additions too... If there are any changes needed, please let me know!

• primitive types are added by default so you can just specify a type="int32" without having to specify another full type for primitives in the XML
• description attribute is optional as allowed by sbe standard
  • message types are also now based off name instead of description
• removed the header_size field that was automatically added in every message
  • this seems to be a cme sbe specific thing
  • i left the current functionality in place by default and you can just pass a an include_message_size_header=False parameter when call SBESchema().parse(), if you're like me and don't want that for your usage of sbe
• blockSize is optional, if not provided block_header will be automatically determined based on the length of the fields
• SBEMessageFactory was broken into a base class and MDPMessageFactory was added for working with cme MDP messages
  • this is related to the same header_size item that seems CME specific the original message factory was accounting for an extra 2 bytes when trying to get the templateId
  • since header_size was cme specific, the SBEParser was updated and expects the build method to return a tuple of (message, message_size)

[tfgm-bud]

Thanks for the contribution @ryanohnemus! I'll review and try to get it merged in within the next day or two.

[tfgm-bud]

@ryanohnemus

In hindsight, using the "description" as the "name" was probably not a great idea. It just so happened that the CME put the TID in the name of their messages and we used the description to make the name more friendly. For example, the CME's AdminHeartbeat message's name is "AdminHeartbeat12" and the description is "AdminHeartbeat". Argh! Your approach is definitely the "right" way to do it.

I have a minor concern that this will break a couple of applications downstream. What would you think about having a use_description_as_message_name flag or something like that for the schema to retain the old behavior? I'm fine with the default being False. I do know of downstream code code that relies on the message names being derived from the description and that would make it pretty easy to fix up these applications. Especially if the MDPMessageFactory() toggled use_description_as_message_name flag to true automatically.

A couple of other things. There are some test cases (./tests) that are broken with this change. Also, there is also an impact is to the SBE class generator in the ./generator subdirectory which is not accounted for. It needs a --message-factory=MDP or something to select the MDPMessageFactory() and a way to deal with the name vs description issue.

[ryanohnemus]

@tfgm-bud can I set include_message_size_header=False by default as well since this seems like a cme/mdp specific thing, and to me it would be nice to just have generic SBE be all the defaults?

I'll go fix the tests, generator, and scripts accordingly.

I'm a little confused on what you said about needing a message-factory with the generator script. I didn't see anything regarding MessageFactory stuff in that folder, everything was just using the SBESchema, but I did miss updating these in my earlier commits... so I can fix these based on my generic/defaults proposal.

Thanks!

[tfgm-bud]

@ryanohnemus sorry, you are right, the factory is only used to decode messages so the generator doesn't need it. Sorry about the confusion there.

[ryanohnemus]

Hey @tfgm-bud! I changed a few things in the last commit, but i'm hoping it's all acceptable!

SBESchema init now has the following parameters

• include_message_size_header, default: False if you are using CME MDP or another SBE type that expects a message_size header field of size uint16 at the beginning of each message, set this to True
• use_description_as_message_name, default: False this is for backwards compatibility, set this to True if you expect the description of the message to be used as the generated class name

The reason I defaulted include_message_size_header to False is because the current behavior seems very CME specific, and I'd like to have the SBESchema defaults be as close to the base standard as possible

I updated all the files accordingly (the tests worked from my machine, let me know if you run into any problems 😄) and I put a note in the README.md about these SBESchema fields

[ryanohnemus]

@tfgm-bud do you have any updates for this PR? thank you!

[ryanohnemus]

also ignore my question about marshaling (i updated/removed the comment)... I should have just read the bottom of the README.md where it mentions using the generated classes... sorry about that!

[tfgm-bud]

Thanks @ryanohnemus. I'll review this early next week and get it checked in.

[tfgm-bud]

@ryanohnemus Sorry for the delay. One more minor issue. In scripts/mdp_book_builder.py, we are still using SBEMessageFactory() it needs to import and use the new MDPMessageFactory(). Make that change and I'll merge. Thanks again. -- Bud

[ryanohnemus]

@tfgm-bud no problem! I updated mdp_book_builder.py.

I have 2 more questions to follow up after this PR:

1. the classes generated with generator do not support subgroups. However if you are using the non-generated SbeSchema.parse you'll be able to parse messages that have groups with subgroups.

Would anyone be able to fix the generator to support groups in groups?

2. will this package ever be available on the public pypi.org / pypi.python.org?

Thank you!

[tfgm-bud]

@ryanohnemus - Can you open tickets for the last two items? Neither looks too bad to implement.

BTW, If you need to pip install, you can do:

pip install git+https://github.com/tfgm/sbedecoder

for master or if you want a specific tag:

pip install git+https://github.com/tfgm/sbedecoder@0.1.2

[ryanohnemus]

ok thank you! I'll open tickets for those.