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

Frozen versions of WES spec #134

Open
susheel opened this issue May 9, 2019 · 10 comments
Open

Frozen versions of WES spec #134

susheel opened this issue May 9, 2019 · 10 comments

Comments

@susheel
Copy link
Member

susheel commented May 9, 2019

To provide backward compatibility support WES servers/clients, we need to provide a frozen-in-time version of the spec. Suggest we keep a major-minor-version in the /openapi folder for the repo locked for any further edits.

WES v1 --> /openapi/v1/workflow_execution_service.swagger.yaml
WES latest --> /openapi/workflow_execution_service.swagger.yaml

OR

WES v1 --> /openapi/workflow_execution_service.v1.swagger.yaml
WES latest --> /openapi/workflow_execution_service.swagger.yaml

P.S. I would also suggest dropping swagger from the specification filename

@jaeddy
Copy link
Member

jaeddy commented May 9, 2019

I like the first option. When you say major-minor, do you mean there might be a /openapi/v1.1/workflow_execution_service.swagger.yaml as well?

I have no objections to dropping swagger from the filename. Is there any convention or standard we should follow?

@susheel
Copy link
Member Author

susheel commented May 10, 2019

I don't think there is a convention. I was suggesting v1 and v1.1, but happy to limit to just major releases.

@geoffjentry
Copy link
Contributor

IMO the general convention is vMAJOR, not vMAJOR.MINOR in the URL

@jaeddy jaeddy added this to the WES v1.1 milestone May 13, 2019
denis-yuen added a commit to ga4gh/tool-registry-service-schemas that referenced this issue May 15, 2019
@susheel
Copy link
Member Author

susheel commented May 21, 2019

Happy with vMAJOR

@jaeddy
Copy link
Member

jaeddy commented May 21, 2019

Sounds good. @susheel, can you make a PR for this?

denis-yuen added a commit to ga4gh/tool-registry-service-schemas that referenced this issue Jun 12, 2019
@juhtornr
Copy link

I think that we should fix #139 before freezing the spec.

garyluu added a commit to ga4gh/tool-registry-service-schemas that referenced this issue Feb 6, 2020
Changes from 2.0.0 + CI fix:

* Explicitly define type in definitions

* Change properties to snakes case and endpoints to camelcase

* Move enum to its own definition

* Incrementing version due to breaking changes

* New endpoint and object definition to describe available files

* For issues #21 and #22

* Renamed File definition to ToolFile due to swagger editor

* Increment version in comment

* Add more file types, better descriptions

* Tool checker proposal (#26)

From https://docs.google.com/document/d/1PTge27WBOKCiR2MkVSOvWKUwnFk0kx-bjq39aKRQxcY/edit?usp=sharing

* Generalize the TRS to account for Singularity and Nextflow
* Change toolversion's boolean indicator

* Add terms of service placeholder

* Install swagger2openapi

* Adding operationIds (#33)

See  dockstore/dockstore#1210

* Moved checker to a property in Tool, also added a boolean

* Specify unencoded paths are also allowed

* Feature/search (#48)

* Add option for search for checker workflows
* For #37 , allow meta_version to be optional

ga4gh/workflow-execution-service-schemas#6
ga4gh/workflow-execution-service-schemas@337cd42

* Add alias suggestion from #38
* Standardize file fields
* Simplify standard (#49)

* Feature/link check (#55)

* Create CONTRIBUTING.md (#65)

* Automatically generate docs for all branches and tags

* TRS documentation update (#64)

* Update openAPI3.sh

* Update BasePath (#72)

Updated base path to `basePath: `/ga4gh/trs/v2`

* Update ga4gh-tool-discovery.yaml (#74)

* Updated LICENCE Copyright (#76)

* Include mention to Nextflow (#77)

Mentioning nextflow as long the API is supporting workflows 
based on Nextflow language (NFL) https://github.com/pditommaso/tool-registry-service-schemas/blob/develop/src/main/resources/swagger/openapi.yaml#L741-L744

* Allow for more than one container image (#58)

* fix type, update OpenAPI version

* fix array examples

* Update openapi.yaml

* Rename metadata to match GA4GH PAP (#78)

* Adding zenodo reference as recommended
* Adding contact URL inspired by WES for PAP
From security questionaire

* More convention sync with cloud workstream APIs (#84)

* Working on #82 also see ga4gh/workflow-execution-service-schemas#134
* Working on harmonizing with GA4GH Discovery  https://github.com/ga4gh-discovery/service-info
* Fix description for #85

* WS-2019-0032  WS-2019-0063

* Attempt to fix tooling (#89)

* Update README.md for gitflow (#91)

* Simple PRC feedback PR (#92)

* Feedback from BOSC

* Typo in example (#99)

* Remove service-info to avoid breaking change (#101)

* Move around a few fields (#97)

* test gradle fix for site deployment (#105)

* Optional auth (#102)

* PRC feedback - strict tool versioning for reproducibility (#93)

* Update to match discussion in DRS
checksum object added and tweaked to match ga4gh/data-repository-service-schemas#282

* Corrects link to GA4GH . checksum hash algorithm registry (#106)

* Correcting hash algorithm registry link

* Correcting hash alg registry link

* Fixing unrelated link

* Glitchy badge (#112)

* Update registry.json (#115)

* Fix CI (#123)

Co-authored-by: Denis Yuen <[email protected]>
Co-authored-by: Yasset Perez-Riverol <[email protected]>
Co-authored-by: Chris Llanwarne <[email protected]>
Co-authored-by: Susheel Varma <[email protected]>
Co-authored-by: Paolo Di Tommaso <[email protected]>
Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>
@jaeddy jaeddy removed this from the WES v1.1 milestone Mar 12, 2020
@ruchim
Copy link
Collaborator

ruchim commented Dec 8, 2020

@susheel -- sorry to get to this so late -- does https://github.com/ga4gh/workflow-execution-service-schemas/blob/develop/openapi/workflow_execution_service.openapi.yaml#L5 solve the spirit of this request -- that we track major/minor version along with the exact definition of that spec?

@vinjana
Copy link

vinjana commented Sep 26, 2022

Not sure whether I should open a new issue or just add my comments here. I'll do the latter.

There is this "freeze" request by @susheel . Could he maybe meant that there should be a tag for the 1.0.1 version? This is actually what I would really like to have before implementing this version in our WES. The 1.0.1 version is already deployed as reference documentation -- so I think it is quite official -- but I do not know the exact commit. Maybe it is possible to find some commit that contains exactly the deployed 1.0.1 version and just add a tag to it?

Generally, in terms of governance, I think as a standard it would be appropriate to have a defined release process -- at least concerning the technical side. In my projects, I made good experiences with releasing via commits and configuring my CD such that only correctly tagged versions (e.g. matching a tag pattern) are being deployed. E.g. such tags may be automatically deployed to the reference documentation site.

Finally as a supplement to the discussion about major/minor versions above: I suggest to use Semantic Versioning 2 for the API spec. It seems like you tried to do s.th. like that -- because you have these three component version numbers. However, from 1.0.0 to 1.0.1 new fields were added in the ServiceInfo, and, AFAIK, this should then better released with a minor version bump. My understanding of semantic versioning is that a major version would not be backwards compatible (e.g. remove fields from responses), a minor version would add features to the API (e.g. add fields to responses), and a patch version would just make small corrections that do not add features nor modify the behavior of the existing spec.

Note that with this interpretation, it would be important to include the minor version into the route. For strict client API implementations it would make a difference, because such a client may check whether a response has additional unexpected fields. On the other hand, an implementation of the 1.1.x version may rely on these new fields to be present. Therefore, unless you want to make major releases when adding fields to responses, a two component version tag would be necessary. I did not think this 100% through and have to correct myself: Just the major version in the route is sufficient. The client should of course not be so strict to deny additional fields (because these may occur with semantic versioning for minor bumps), and for determining the minimal version supported by the server, the client can just ask the ServiceInfo.

Is this reasoning correct?

@vinjana
Copy link

vinjana commented Sep 27, 2022

The missing 1.0.1 version tag issue is clearer covered in #191.
Semantic versioning is also covered in #71.

I don't see what other topics of this issue are not better covered by other issues, and propose to close this issue.

@uniqueg
Copy link
Contributor

uniqueg commented Sep 27, 2022

Thanks for pointing me to your comment here, @vinjana, I hadn't seen that.

As for closing this issue: I think the proposal to include frozen/tagged/released versions of the specs in separate files goes beyond what is being described in #191. Having all versions available in the same directory might be more convenient in some cases (e.g., when comparing versions) as opposed to having to check out releases individually. However, it also comes with additional complexity, on the maintenance side, but potentially also on the user/implementer side (we probably wouldn't want new implementers to think much about what version to use, but instead rather pick the latest one).

Personally, I tend to favor having just one file and let the version control take care of what it is designed for. I would, however, like if we included pushing releases to a service like Zenodo (https://zenodo.org/), so that the specs become citable (DOI). So that may potentially be a compromise here?

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

7 participants