Move API clients into separate packages #3124

onursumer · 2020-03-26T01:41:44Z

Separated out OncoKB and Genome Nexus API clients into their own packages. Now we can publish these API clients as standalone packages.

Checks

Has tests or has a separate issue that describes the types of test that should be created. If no test is included it should explicitly be mentioned in the PR why there is no test.
The commit log is comprehensible. It follows 7 rules of great commit messages. For most PRs a single commit should suffice, in some cases multiple topical commits can be useful. During review it is ok to see tiny commits (e.g. Fix reviewer comments), but right before the code gets merged to master or rc branch, any such commits should be squashed since they are useless to the other developers. Definitely avoid merge commits, use rebase instead.
Is this PR adding logic based on one or more clinical attributes? If yes, please make sure validation for this attribute is also present in the data validation / data loading layers (in backend repo) and documented in File-Formats Clinical data section!

onursumer · 2020-03-26T16:12:44Z

packages/genome-nexus-ts-api-client/package.json

+{
+  "name": "genome-nexus-ts-api-client",
+  "description": "Genome Nexus API Client for TypeScript",
+  "version": "0.1.0",


This version should reflect the actual API version, ideally the latest tag version on genome-nexus master branch.

setting this to 1.1.2-beta.0 for now. latest master is 17 commits ahead of the latest release 1.1.1 so it is better set it to the next beta version.

onursumer · 2020-03-26T16:13:09Z

packages/oncokb-ts-api-client/package.json

+{
+  "name": "oncokb-ts-api-client",
+  "description": "OncoKB API Client for TypeScript",
+  "version": "0.1.0",


This version should reflect the actual API version, ideally the latest tag version on oncokb master branch

after discussing with @zhx828, decided to set the version to the current version in swagger doc, which is 1.0.0

onursumer · 2020-03-31T21:47:37Z

package.json

@@ -1,6 +1,7 @@
 {
  "name": "cbioportal-frontend",
  "private": true,
+  "version": "3.2.13-alpha.0",


added version back to make updatePackageVersion also update the root package.json dependencies. If the version is missing, lerna ignores this file completely when updating other package versions.

when do we need to update this? In every PR?

It is not critical to update this version every time. The main reason to have a version in this package.json is to make lerna version to properly update dependencies. It would be nice to keep this version in sync with the tagged master version though.

onursumer · 2020-03-31T21:50:21Z

README.md

@@ -336,8 +336,6 @@ then update all package versions with:
 yarn run updatePackageVersion
 ```

-After that update the versions of all packages in the root `package.json`


We don't need to do this anymore, instead we are adding version back to the root package.json and let lerna version handle dependencies automatically like it does with all other versioned packages.

zhx828

Finally！ Nice work! Thanks。

onursumer · 2020-03-31T23:28:18Z

@inodb @alisman @zhx828 cBioPortal API still lives under src/shared/api/generated. Would it be useful to have a separate package for it as well?

inodb · 2020-04-01T01:05:00Z

@onursumer potentially, I guess most times you would want to use the API + components. But maybe it's useful to allow API access as well without using mobx + components. If it's not extra maintenance effort then why not I guess

inodb

Nice! 🎉

Just one question re: when to update the root package version. For the other clients it's obvious I think, whenever the API updates we should update the client and all packages depending on that client. But for the root package it's not that obvious when we update that number? Whenever we tag a release or something?

onursumer · 2020-04-01T20:07:24Z

@inodb ideally root package.json should match the frontend tag on master. But, we don't really need to update package.json verison since it is a private package that we never publish. It is good enough to update the dependencies on the root package.json.

We can even remove the version from the root package.json, but then we need to update dependencies manually since lerna version ignores the root package when there is no version field. It's okay if the root package.json version is out of sync, for now we don't really use that version anywhere.

onursumer · 2020-04-06T22:57:59Z

package.json

@@ -131,7 +132,8 @@
    "bootstrap-sass": "3.3.7",
    "bowser": "^1.7.1",
    "bundle-loader": "^0.5.4",
-    "cbioportal-frontend-commons": "0.2.6",
+    "cbioportal-frontend-commons": "^0.2.7",
+    "cbioportal-ts-api-client": "^1.0.0-beta.0",


Current API version in swagger doc is 1.0 (beta)

Signed-off-by: Onur Sumer <s.onur.sumer@gmail.com>

Move API clients into separate packages Former-commit-id: 265dfcc

onursumer force-pushed the modular-api-clients branch from f20e882 to c2e2470 Compare March 26, 2020 14:31

onursumer commented Mar 26, 2020

View reviewed changes

onursumer force-pushed the modular-api-clients branch 5 times, most recently from e9b69fa to 0123ec6 Compare March 31, 2020 21:42

onursumer commented Mar 31, 2020

View reviewed changes

onursumer requested review from zhx828, inodb and alisman March 31, 2020 21:48

onursumer marked this pull request as ready for review March 31, 2020 21:48

onursumer commented Mar 31, 2020

View reviewed changes

zhx828 approved these changes Mar 31, 2020

View reviewed changes

inodb approved these changes Apr 1, 2020

View reviewed changes

onursumer force-pushed the modular-api-clients branch 8 times, most recently from ad5ab19 to c730f6b Compare April 6, 2020 22:44

onursumer commented Apr 6, 2020

View reviewed changes

onursumer force-pushed the modular-api-clients branch 2 times, most recently from 1066c6e to ecdfd02 Compare April 7, 2020 18:42

factor out GenomeNexus API client to a separate package

884d50e

Signed-off-by: Onur Sumer <s.onur.sumer@gmail.com>

onursumer added 2 commits April 7, 2020 15:30

factor out OncoKB API client to a separate package

e5f3009

Signed-off-by: Onur Sumer <s.onur.sumer@gmail.com>

factor out cBioPortal API client to a separate package

9bf8a68

Signed-off-by: Onur Sumer <s.onur.sumer@gmail.com>

onursumer force-pushed the modular-api-clients branch from ecdfd02 to 9bf8a68 Compare April 7, 2020 19:31

onursumer merged commit 265dfcc into cBioPortal:master Apr 7, 2020

onursumer deleted the modular-api-clients branch April 7, 2020 21:16

inodb added the devops label Apr 9, 2020

inodb pushed a commit to inodb/cbioportal-frontend that referenced this pull request Jan 12, 2022

Merge pull request cBioPortal#3124 from onursumer/modular-api-clients

b1f410f

Move API clients into separate packages Former-commit-id: 265dfcc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move API clients into separate packages #3124

Move API clients into separate packages #3124

onursumer commented Mar 26, 2020 •

edited

onursumer Mar 26, 2020

onursumer Apr 6, 2020

onursumer Mar 26, 2020

onursumer Apr 6, 2020

onursumer Mar 31, 2020

inodb Mar 31, 2020

onursumer Apr 6, 2020

onursumer Mar 31, 2020 •

edited

zhx828 left a comment

onursumer commented Mar 31, 2020

inodb commented Apr 1, 2020

inodb left a comment

onursumer commented Apr 1, 2020

onursumer Apr 6, 2020

Move API clients into separate packages #3124

Move API clients into separate packages #3124

Conversation

onursumer commented Mar 26, 2020 • edited

Checks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

onursumer Mar 31, 2020 • edited

Choose a reason for hiding this comment

zhx828 left a comment

Choose a reason for hiding this comment

onursumer commented Mar 31, 2020

inodb commented Apr 1, 2020

inodb left a comment

Choose a reason for hiding this comment

onursumer commented Apr 1, 2020

Choose a reason for hiding this comment

onursumer commented Mar 26, 2020 •

edited

onursumer Mar 31, 2020 •

edited