Skip to content
This repository has been archived by the owner on May 23, 2024. It is now read-only.

Remove /${trapiVersion}/ URL prefix #551

Open
gaurav opened this issue Aug 9, 2022 · 4 comments
Open

Remove /${trapiVersion}/ URL prefix #551

gaurav opened this issue Aug 9, 2022 · 4 comments
Assignees

Comments

@gaurav
Copy link
Member

gaurav commented Aug 9, 2022

Several pieces of code establish a /${trapiVersion}/ URL prefix for the CAM-KP-API server:

sttp.tapir.openapi
.Server(s"${appConfig.location}/${appConfig.trapiVersion}")
.description("Default server")
.extensions(ListMap("x-maturity" -> ExtensionValue(s"${appConfig.maturity}"), "x-location" -> ExtensionValue("RENCI")))

uri = Uri.fromString(s"https://cam-kp-api.renci.org/${appConfig.trapiVersion}/query").toOption.get

https://github.com/helxplatform/translator-devops/blob/1db3a7cffdb2b28ce3970b1f3a09b28769cc62e1/helm/cam-kp-api/templates/deployment.yaml#L27-L28

I think this was intended to ensure that we could host multiple TRAPI versions simultaneous, but I don't think we plan to do that. It therefore creates additional work for us as we move to TRAPI v1.3, since our server endpoint will change to /1.3/ instead of /1.2/ and we will need to re-register all our endpoints with SmartAPI (#550).

Is there any other reason for keeping this prefix? If not, I'll go ahead and remove them from our code.

@gaurav gaurav self-assigned this Aug 9, 2022
@jdr0887
Copy link
Collaborator

jdr0887 commented Aug 11, 2022

Yes, we want to keep that. As new trapi releases are made, Translator wanted to be able to have deployments that can transition from one to the next. It is my understanding that all KP's & such are expected to rollout trapi specific deployments of their code upon each new trapi release. It is extra work to re-register the endpoints.

@balhoff
Copy link
Contributor

balhoff commented Aug 12, 2022

But I think either way we need to register different endpoints. The different URLs with different TRAPI versions are different locations. As far as I'm aware there isn't a standardized API for changing TRAPI version. I think it would simplify things to remove this part of the path.

@gaurav
Copy link
Member Author

gaurav commented Aug 15, 2022

It seems to me like we have at least three options here:

  1. Currently, we publish different Docker images for e.g. cam-kp-api:0.1.2-trapi-1.2 and cam-kp-api:0.1.2-trapi-1.3. We then use Kubernetes to host these images at different locations (in this case, https://cam-kp-api.renci.org/1.2/ and https://cam-kp-api.renci.org/1.3/). We could continue to do this -- we would simultaneously maintain 0.1.*-trapi-1.2 and 0.1.*-trapi-1.3, and then drop support for TRAPI 1.2 once it has been deprecated.
    • In order to make this work, the Scala code will read the trapiVersion value from application.conf or the environment and activate or deactivate TRAPI features as needed. However, we could simplify this work a bit by freezing the code at a particular version -- for example, 0.1.2-trapi-1.2 could be the last version to support TRAPI 1.2, and all additional features will only be available in the TRAPI 1.3 version of our code.
  2. My proposal above was to get rid of this distinction entirely: we provide a single endpoint at https://cam-kp-api.renci.org/ that tells the user which single TRAPI version it supports. The main advantages are simplicity and reduced maintenance burden. However, as Jason pointed out, this means that ARAs that only support TRAPI 1.2 will no longer be able to query CAM-KP-API once we upgrade to TRAPI 1.3.
  3. Another possibility that I think would simplify deployment a bit would be to publish a single application (cam-kp-api:0.1.2) to a single endpoint (e.g. https://cam-kp-api.renci.org/) and then to have that single application create multiple endpoints for each TRAPI version (/1.2/... for TRAPI 1.2, /1.3/... for TRAPI 1.3 and so on). I like this approach because the code organization would be very straightforward here: instead of a single Server class, we would have a TRAPI12Server, a TRAPI13Server, and so on, all inheriting from a single Server class that includes all the shared code, and then each class could extend the previous class and modify it as needed. Once a TRAPI version was deprecated, its code could be moved into the Server class and removed as a separate endpoint.

Jim, Jason: Are these all the options we're considering, or is there a better option I'm missing? Which ones do y'all think would work best?

@gaurav
Copy link
Member Author

gaurav commented Aug 19, 2022

For the TRAPI 1.3 release, I'm going to go with a modified version of option 1 above:

  • I've made a branch for cam-kp-api-0.1.2-trapi-1.2 in this repository. In the Git Flow terminology, this is the "master" branch -- until TRAPI 1.2 is deprecated, we can apply hotfixes to this branch as needed.
  • I'll continue developing CAM-KP-API 0.1.3 on the master branch (the develop branch in Git Flow terms) which will only support TRAPI 1.3 -- there will never be a TRAPI 1.2 release for CAM-KP-API 0.1.3.

I considered going full Git Flow and having a separate master branch (for CAM-KP-API 0.1.2) and develop branch (for CAM-KP-API 0.1.3), but our current Github Flow approach seems to be working well for everybody, so let's not change that unless we have to.

We can keep this approach for TRAPI 1.4 and beyond until we run into a problem with this approach, and which point we can reconsider the three options listed above (or any new options that become available to us then).

I'll go ahead and close this issue with this resolution, but if anybody has any objections, please feel free to reopen it!

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

No branches or pull requests

3 participants