cube icon indicating copy to clipboard operation
cube copied to clipboard

feat: add base version of cockroach driver

Open frolovdev opened this issue 2 years ago • 2 comments

Check List

  • N/A Tests has been run in packages where changes made if available
  • [x] Linter has been run for changed code
  • [x] Tests for the changes have been added if not covered yet
  • N/A Docs have been added / updated if required

Issue Reference this PR resolves

https://github.com/cube-js/cube.js/issues/1164

Description of Changes Made (if issue reference is not provided)

I guess it's a good starting point for supporting cockroach, wait for any feedback or next steps guys (I just propose a draft, because maybe right now it's not a big deal to rewrite it if I'm moving toward wrong way)

I mostly take code from the Postgres plugin, I don't extend the original functionality because it's a community-driven plugin right now, so some copy-paste looks like a reasonable thing to avoid a bunch of tension that can be caused by inheritance.

Some parts of pr will be covered by self-review comments 🎵

frolovdev avatar Jun 19 '22 18:06 frolovdev

Codecov Report

Merging #4773 (b2361e9) into master (d444c0f) will decrease coverage by 13.22%. The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #4773       +/-   ##
===========================================
- Coverage   72.41%   59.19%   -13.23%     
===========================================
  Files         254      136      -118     
  Lines       27624    11187    -16437     
  Branches     2687     2781       +94     
===========================================
- Hits        20004     6622    -13382     
+ Misses       7319     4060     -3259     
- Partials      301      505      +204     
Flag Coverage Δ
cube-backend 59.19% <ø> (+0.04%) :arrow_up:
cubesql ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../cubejs-server-core/src/core/DriverDependencies.js 100.00% <ø> (ø)
packages/cubejs-api-gateway/src/jwk.ts 11.47% <0.00%> (ø)
packages/cubejs-backend-shared/src/env.ts 31.66% <0.00%> (ø)
packages/cubejs-api-gateway/src/gateway.ts 69.23% <0.00%> (ø)
packages/cubejs-api-gateway/src/graphql.ts 4.16% <0.00%> (ø)
packages/cubejs-backend-shared/src/time.ts 28.57% <0.00%> (ø)
packages/cubejs-backend-shared/src/proxy.ts 23.52% <0.00%> (ø)
packages/cubejs-backend-shared/src/track.ts 29.72% <0.00%> (ø)
packages/cubejs-backend-shared/src/errors.ts 28.57% <0.00%> (ø)
packages/cubejs-api-gateway/src/sql-server.ts 6.45% <0.00%> (ø)
... and 144 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0ac1f95...b2361e9. Read the comment docs.

codecov[bot] avatar Jun 26 '22 18:06 codecov[bot]

Hey @frolovdev. Nice to meet you. It's an excellent starting point to add the CockroachDB driver to Cube.js. There are some notes from my side, mostly around tests. I'm also concerned about adding this driver to the e2e test suite here cubejs-testing/test/driver-test-suite.ts. But in general, you did a great job! Thanks a lot!

buntarb avatar Jul 11 '22 15:07 buntarb