Skip to content

Conversation

@patmagee
Copy link
Contributor

@patmagee patmagee commented Jun 8, 2023

Motivation

This release focuses on two main themes: Observability and Scalability.

Observability: As more people have adopted WES it has become clear that the current API does not provide enough information for end users. The RunStatus and Log models have limited required keys and making actionable decisions on the information that is available is difficult without performing additional requests.

Scalability: As more implementors are looking to "productize" WES, the topic of scalability has continuously come up. Specific features of WES (such as embedded task logs) make implementing WES API a challenge, and can cause the service to break (Ie too many task cause a service to consume too much memory).

What's Changed

Additional Improvements

  • Removed travis CI integration and added GitHub actions for publishing docs and linting openapi.yaml
  • Removed example python client code in b5f782e
  • Removed duplicated swagger specification in favour of consolidated OpenAPI3 specification

jaeddy and others added 30 commits June 22, 2020 15:56
explode out openapi spec to use multifile organization
Remove swagger Definition and Reference Python Code
Switch to github actions and add Open API lining and validation
Updated requested permissions for doc build
Changed actions to use pull_request_target
* Add in PREEMPTED.

* Resolve reorganization of yaml's into 1 single yaml.
…a request. Closes #181 (#182)

* Add in support for workflow enginer and version when submitting a request.

* Expand out Workflow Engine version to by array of strings.

* Update RunRequest.yaml

* Add in workflow_engine and version to the run spec.

* Fix typo

* Fix for consitency

* Add more clarity about engine and version

* Update description

* Address changes requested.

* Fix indentation.

* Resolve reorganization of yaml's into 1 single yaml.

* Remove preempted with accidental merge.

* Add required parameter for:
- workflow_type
- workflow_type_version
- workflow_url
* Merge changes from develop

* Removed invalid attribute from openapi definition and aded nullable field instead

* Added explicit deprecation warning on the task_log property
…205)

* Added TaskLog model and created endpoint for retrieving a single task by id

* Added system_logs and tes_uri properties to TaskLog object

* Fixed spec tag metadata to include TaskLog and TaskListResponse

* Updated TaskLog to extend Log, and allow an array of TaskLog items to be returned in the task_log field

* Fixed casing on models and error response description
Copy link
Contributor

@uniqueg uniqueg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for all the work @patmagee, @vsmalladi and others 🙏

@patmagee
Copy link
Contributor Author

@briandoconnor @dglazer this rc has been open for a while. What are the next steps to getting this ratified as versions 1.1 of Wes and released?

@uniqueg
Copy link
Contributor

uniqueg commented Aug 15, 2023

I may be wrong here, so would indeed be good to have confirmation from @dglazer and/or @briandoconnor.

However, I do think that if these changes have been shared with the community and the community has been given ample opportunity to comment, raise concerns, veto etc., and if that is all fine, I think you should be good to go. As far as I know, minor updates do not require SC approval and no particular paperwork.

So I'd say just tag the head commit with the new version, push the tag and create a release through the GitHub interface.

@dglazer
Copy link
Member

dglazer commented Aug 15, 2023

Can you confirm this is a backwards-compatible change, specifically meaning that old clients will work fine with new servers? (The other way around is also important, but as long as new clients can check the server version and fall back appropriately it's manageable.) If so, I mostly agree with @uniqueg that no extra process is needed.

Re "shared with the community" -- I agree this has been out there for a while. @briandoconnor , do you think that's sufficient, or should we do one more broad "release coming in two weeks" email? (I'm 70/30 in favor of just doing it, given previous communication.)

@uniqueg
Copy link
Contributor

uniqueg commented Aug 15, 2023

Without a comprehensive compliance test suite for clients, it's virtually impossible to guarantee that any old client would still work.

For example, consider a response model where additionalProperties is set to True (which is generally the case for most, if not all Cloud API response models; it's the default) but a client falsely sets it to False for validation of that model. Adding a new property (not a breaking change) would break that client, although that client worked just fine before and seemed compliant. This is the client's fault and shouldn't stop us from adding properties.

In terms of checks, I think WES now uses, in its CI, a library that checks the API changes for breaking changes automatically. In addition, I believe that all PRs have been reviewed carefully to ensure backwards compatibility in both directions in theory.

I do think we should work towards better automated testing though. Test suites for servers are now available for DRS and TES, and are in the making for WES. However, these are still incomplete (compliance tests are +/- okay, but functional tests are largely still missing). Test suites for clients, as far as I know, are not available (or even under development?) for any of the Cloud APIs.

@patmagee
Copy link
Contributor Author

@dglazer as far as I am aware, none of the changes introduced are breaking and are fully backwards compatible. As @uniqueg pointed out, this is functionally hard to test without a complete compliance test suite.

Once @briandoconnor weighs in here, I think we are good to mint the release! @dglazer who is responsible for publishing releases on Zenodo?

@patmagee patmagee closed this Aug 22, 2023
@patmagee patmagee reopened this Aug 22, 2023
@patmagee patmagee merged commit 7ffbfed into master Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants