workflow-execution-service-schemas
workflow-execution-service-schemas copied to clipboard
Document versioning
The current versioning scheme does not follow semantic versioning. We should move towards a semantic version to make it clear when there are breaking changes.
@david4096 — are you and @natanlao automating this as part of the build process (e.g., with something like bumpversion? Seems like it might be nice, especially for patches.
We don't really have a clear concept of "patch" vs. "minor" version at the moment, but I think we're probably OK with clarifying breaking changes moving forward. Need to think about how to address this in contributing docs.
@jaeddy -- I almost feel this request is more of an implementation detail for spec maintenance. It seems manually updating has worked out well enough in the past 2 years -- I'd be curious for your 2 cents on looking back, if you think adding a bumpversion would've helped catch any human errors or simplified the process of versioning?
@david4096 -- do you feel https://github.com/ga4gh/workflow-execution-service-schemas/blob/develop/openapi/workflow_execution_service.openapi.yaml#L5 gets to the right level of semantic versioning?
I think this is an issue relevant to all Cloud WS APIs and possibly other WS as well.
Two points from my side in favor of adopting fully SemVer-compliant versioning, one technical, one more "ideological":
- When implementing WES and other Cloud WS APIs, I occasionally ended up having to check the commit SHA & history to see if I'm still working with the latest version, because that info cannot be parsed from the specs themselves if there's no bump in the version. This is tedious and error-prone and difficult to automate.
- As a standard-setting organization, I feel the GA4GH would do well in leading by example when it comes to adopting other widely used community standards, such as SemVer.
When it comes to bumping versions, SemVer stipulates the following:
- MAJOR version when you make incompatible API changes,
- MINOR version when you add functionality in a backwards compatible manner, and
- PATCH version when you make backwards compatible bug fixes.
For the Cloud WS APIs at least, I would propose to translate this in the following way:
- Increment MAJOR version for breaking changes to the API
- Increment MINOR version for backwards-compatible changes to the API (e.g., add a new endpoint or add an optional property to a schema used in an existing endpoint)
- Increment PATCH version for any changes that do not alter the behavior of the API (e.g., refactoring OpenAPI document, update descriptions/documentation)
To me this seems pretty much non-ambiguous. Tooling could be used to bump versions automatically, given a keyword in the commit message(s) (adopting Convential Commits may allow us to use existing tooling) associated with a given PR.
Note that adding an optional field in an expected request is backwards compatible. In a response, however, a simple non-optional field may also be considered backwards compatible, if it is assumed that the client is non-strict and ignores additional fields.
I think this assumption is reasonable, because a client that is so strict as to complain about added fields in responses will not be forward-compatible for an API that follows SemVer2 versioning. I bet client implementors will want to avoid an unnecessary exact version lock-in, but rather profit from the semantic versioning scheme.
There is (now) tooling for classifying breaking and non-breaking changes in OpenAPI: https://bitbucket.org/atlassian/openapi-diff/src/master/
The underlying classification rules are:
Breaking Changes that would make existing consumers incompatible with the API, for example:
- removing a path
- removing a method
- changing a property from optional to required in a request body
- changing a property from required to optional in a response body
Non Breaking Changes that would not make existing consumers incompatible with the API, for example:
- adding a path
- adding a method
- changing a property from required to optional in a request body
- changing a property from optional to required in a response body
Unclassified Changes that have been detected by the tool but can't be classified, for example:
- modifications to X-Properties
We have recently included openapi-diff in the CI for TES: https://bitbucket.org/atlassian/openapi-diff/src/master/
Note that this can only assist in deciding which version to bump, because:
- The tool does not differentiate between "minor" and "patch" (e.g., typos in descriptions, exploding models) changes
- It may not currently be possible to specify certain complex requirements in OpenAPI 3.x (or weren't in OpenAPI 2.x and the specs have not yet been updated since migrating)
- Not all features of the latest OpenAPI version may be supported by this and other tools (e.g., validators, code generators), which might lead to things breaking even for "non-breaking" changes
- Any other requirements "hidden" in descriptions (of which there are, unfortunately, quite a few across the Cloud APIs)