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 package scoping for types section to override schema package #507

Closed
lapetiteclef opened this issue Aug 30, 2017 · 6 comments
Closed

Comments

@lapetiteclef
Copy link

lapetiteclef commented Aug 30, 2017

We are using quite many types (some large enums with structured error messages, nested composites etc) to get common reused parts. To get code reuse we need to get types from several schemas in the same package. I wonder how other solve similar problems? Its basically traditional canonical model upstreams and adapters conforming to it. We are using preprocessing of xml to get nice set of schema files and categories but that only put structure the SBE code genaration input. This is for the output.

I tested and added support for a new package attribute to the type section locally. It worked well generating fully qualified types into separate package.
Example

`<types package="test.api.common">`
`</types>`
`<types package="test.api.common.product">`
`</types>`

The resulting code reuse in SBE generated code is not important because its generated, but all surrounding transformations / code managing SBE is not. This feature reduces code duplication in developer written code if shared types are used across schemas.

Using an optional package attribute would be backwards compatible. Also the SBE wireformat stays the same. The feature would only make sharing of types possible. Now the only way to share types is to point several schemas to the same output package. Redistribution of SBE APIs using just one protocol will then be difficult.

In our large project I think this or similar feature is a must. We have to continue with existing branch in worst case. Its not too bad branching when its wire compatible I guess. Its easier for us to clean out the package attribute in the types section (when integrating with 3rd party over SBE) than maintaining code duplication for same types for many adapters in the system.

Any input on this subject is greatly appreciated.

@mjpt777
Copy link
Contributor

mjpt777 commented Aug 30, 2017

This is a change to the spec. Please raise it with the spec body and see if they will include it for a future release.

https://github.com/FIXTradingCommunity/fix-simple-binary-encoding

@mjpt777 mjpt777 closed this as completed Aug 30, 2017
@mjpt777 mjpt777 changed the title feature request Add package scoping for types section to override schema package Aug 30, 2017
@lapetiteclef
Copy link
Author

Thanks for the info, I did not know spec and implementation was separated community.

@mjpt777
Copy link
Contributor

mjpt777 commented Sep 1, 2017

You're welcome. We are very happy to receive suggestions for improving our implementation but are limited in what we can do regarding the specification.

@ratcashdev
Copy link
Contributor

ratcashdev commented Jul 1, 2022

with regards to FIXTradingCommunity/fix-simple-binary-encoding#96 and the commit in v2-RC2, can we have an option switch, like below, and support for such type elements? If yes, I'd volunteer to submit a PR.

/**
     * Boolean system property to control the support of package names in {@code <types>} elements.
     * Part of SBE v2-rc2. Defaults to false.
     */
    public static final String VERSION_2_AWARE = "sbe.v2.aware";

@mjpt777
Copy link
Contributor

mjpt777 commented Jul 2, 2022

Rather than something like sbe.v2.aware it may be more suitable to have something like sbe.type.package.override to allow the pull in of a feature. If you create a PR that is comprehensive and forward compatible then we would consider it.

@ssh352
Copy link

ssh352 commented Jul 9, 2022

thought about this too. would be very useful.

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

No branches or pull requests

4 participants