finos / common-domain-model

The CDM is a model for financial products, trades in those products, and the lifecycle events of those trades. It is an open source standard that aligns data, systems and processes and is available as code in multiple languages for easy implementation across technologies.
Other
124 stars 53 forks source link

[DEV] DSL update #2969

Closed SimonCockx closed 2 months ago

netlify[bot] commented 3 months ago

Deploy Preview for finos-cdm ready!

Name Link
Latest commit 344d865c5b5972911cde642d42233a8aeeccf7ac
Latest deploy log https://app.netlify.com/sites/finos-cdm/deploys/6661bd1190576600089e5668
Deploy Preview https://deploy-preview-2969--finos-cdm.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

minesh-s-patel commented 3 months ago

This appears to make a change to the DSL that should be approved by the TAWG before the CDM is updated to use it. The TAWG is meeting next week, please present the change and its impact at that meeting.

We did not get a chance to discuss this in the TAWG. @chrisisla and all - Can you please see https://github.com/finos/rune-dsl/issues/747? It has the audit trail of the original requirements and thought process. Note that we do not have to use the new syntax in the CDM until its use is ratified by the TAWG.

I would really like to get this in. Any updates rune-dsl, rosetta-code-generators or rosetta-common changes cannot go into CDM until this is merged.

There are concerns about the potential of misuse of the new syntax when it comes to compatibility between minor versions. We already have backwards compatibility guidelines that I think covers that case - we can modify if we see fit.

Backward Compatibility

Like other types of software, backward compatibility in the context of a domain model means that an implementor of that model would not have to make any change to update to such version.

  • Prohibited changes:
    • Change to the structure (e.g. the attributes of a data type or the inputs of a function) or removal of any model element
    • Change to the name of any model element (e.g. types, attributes, enums, functions or reporting rules)
    • Change to any condition or cardinality constraint that makes validation more restrictive
    • Change to the DSL that results in any existing expression becoming invalid
    • Change to the DSL that results in change to any of the generated code's public interfaces
  • Allowed changes:
    • Change that relaxes any condition or cardinality constraint
    • Change to any synonym that improves, or at least does not degrade, the mapping coverage
    • Addition of new examples or test packs
    • Change to the user documentation or model descriptions
    • Addition of new data types, optional attributes, enumerations, rules or functions that do not impact current functionality
minesh-s-patel commented 3 months ago

This appears to make a change to the DSL that should be approved by the TAWG before the CDM is updated to use it. The TAWG is meeting next week, please present the change and its impact at that meeting.

We did not get a chance to discuss this in the TAWG. @chrisisla and all - Can you please see finos/rune-dsl#747? It has the audit trail of the original requirements and thought process. Note that we do not have to use the new syntax in the CDM until its use is ratified by the TAWG.

I would really like to get this in. Any updates rune-dsl, rosetta-code-generators or rosetta-common changes cannot go into CDM until this is merged.

There are concerns about the potential of misuse of the new syntax when it comes to compatibility between minor versions. We already have backwards compatibility guidelines that I think covers that case - we can modify if we see fit.

Backward Compatibility

Like other types of software, backward compatibility in the context of a domain model means that an implementor of that model would not have to make any change to update to such version.

  • Prohibited changes:

    • Change to the structure (e.g. the attributes of a data type or the inputs of a function) or removal of any model element
    • Change to the name of any model element (e.g. types, attributes, enums, functions or reporting rules)
    • Change to any condition or cardinality constraint that makes validation more restrictive
    • Change to the DSL that results in any existing expression becoming invalid
    • Change to the DSL that results in change to any of the generated code's public interfaces
  • Allowed changes:

    • Change that relaxes any condition or cardinality constraint
    • Change to any synonym that improves, or at least does not degrade, the mapping coverage
    • Addition of new examples or test packs
    • Change to the user documentation or model descriptions
    • Addition of new data types, optional attributes, enumerations, rules or functions that do not impact current functionality

@chrisisla @dschwartznyc

As part of the current standard operating procedures, PR https://github.com/finos/common-domain-model/pull/2984 (Support for mangling reserved Python keywords used in CDM Rune definitions) was merged, approved and released on the 13th Of June 2024.

Since this change was built on the latest version of Rune DSL (as expected as is was developed later), the latest version of the Rune DSL 9.10 is included in that release.

dschwartznyc commented 3 months ago

@minesh-s-patel @chrisisla

REVISED

while very grateful for the expedited Python release of the generator and the ensuing CDM artifacts, could you please clarify whether the newest version of the CDM does in fact include the new version of the DSL.

the parent POM for the generator refers to <rosetta.dsl.version>9.10.0</rosetta.dsl.version>. from my scan of the repo, this version of the DSL includes This release implements step 1 and step 2 of https://github.com/finos/rune-dsl/issues/747#issuecomment-2105050958 (enhanced support for one-of types). for clarity, the Python generator does not implement any of these changes.

on the other hand, the POM for the CDM release specifies <rosetta.dsl.version>9.8.5</rosetta.dsl.version> which is an earlier version.

is it fair to assume that this reference explicitly ties the CDM release to the earlier version of the DSL?

SimonCockx commented 3 months ago

@dschwartznyc

To see how the new DSL got it's way in the CDM, here is a simplified dependency graph:

CDM
\---> Rune DSL 9.8.5
\---> Rune bundle (including code generators) 11.10.0
      \--> Rune DSL 9.10.0

You can see the DSL version specified by the CDM and the DSL version the CDM gets transitively from the bundle are out of sync, which is the cause of this confusion. Due to technical reasons, the effective DSL version that the Rosetta IDE uses is taken from the bundle, which in this case is 9.10.0, the version including the "enhanced support for one-of types". There is already a PR to sync the versions again: https://github.com/finos/common-domain-model/pull/2992.

dschwartznyc commented 3 months ago

@dschwartznyc

To see how the new DSL got it's way in the CDM, here is a simplified dependency graph:

CDM
\---> Rune DSL 9.8.5
\---> Rune bundle (including code generators) 11.10.0
      \--> Rune DSL 9.10.0

You can see the DSL version specified by the CDM and the DSL version the CDM gets transitively from the bundle are out of sync, which is the cause of this confusion. Due to technical reasons, the effective DSL version that the Rosetta IDE uses is taken from the bundle, which in this case is 9.10.0, the version including the "enhanced support for one-of types". There is already a PR to sync the versions again: #2992.

@SimonCockx thanks. Seems odd that Rune is specified both directly and implicitly via the bundle. To clarify, does inclusion via the bundle imply that the new Rune features are available for modeling?

SimonCockx commented 3 months ago

@dschwartznyc Modellers can use the new Rune features in Rosetta, yes.

dschwartznyc commented 3 months ago

@dschwartznyc Modellers can use the new Rune features in Rosetta, yes.

whoops

chrisisla commented 2 months ago

The enhancements to Rune were discussed with a group of CDM and TAWG Maintainers and consensus was reached on accepting the changes for use in the CDM.

lolabeis commented 2 months ago

Closing as superseded by #2992, which updated the dependency to an ulterior version of the DSL.