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

Add support for xt db basis #56

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

naomijub
Copy link
Owner

@naomijub naomijub commented Aug 2, 2023

No description provided.

@naomijub
Copy link
Owner Author

naomijub commented Aug 2, 2023

@alexandrasp You don't need to manually create the protobufs, you can just add them to https://github.com/naomijub/gXTDB/blob/main/resources/service.proto as Request and Response data structures, create a service that handles them then execute make all

- define protobuf for db-basis
- add server and client sides for db-basis

Signed-off-by: Alexandra Pereira <[email protected]>
@alexandrasp alexandrasp force-pushed the add-support-for-xt-db-basis branch from c69cf35 to 8a81110 Compare August 2, 2023 04:29
resources/service.proto Outdated Show resolved Hide resolved
@alexandrasp
Copy link
Collaborator

@alexandrasp You don't need to manually create the protobufs, you can just add them to https://github.com/naomijub/gXTDB/blob/main/resources/service.proto as Request and Response data structures, create a service that handles them then execute make all

quick and easy 🥇

Signed-off-by: Alexandra Pereira <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Aug 2, 2023

Codecov Report

Patch coverage has no change and project coverage change: +0.12% 🎉

Comparison is base (ffe45e3) 70.98% compared to head (6b9ff37) 71.11%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #56      +/-   ##
==========================================
+ Coverage   70.98%   71.11%   +0.12%     
==========================================
  Files           6        6              
  Lines         224      225       +1     
==========================================
+ Hits          159      160       +1     
  Misses         65       65              
Files Changed Coverage Δ
gxtdb-rs/tests/mock.rs 80.00% <0.00%> (-2.76%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alexandrasp alexandrasp force-pushed the add-support-for-xt-db-basis branch 2 times, most recently from 98e45aa to 91369cb Compare August 8, 2023 12:45
@alexandrasp alexandrasp force-pushed the add-support-for-xt-db-basis branch from 91369cb to cd8cc94 Compare August 9, 2023 04:31
@alexandrasp alexandrasp self-assigned this Aug 9, 2023
@alexandrasp alexandrasp linked an issue Aug 9, 2023 that may be closed by this pull request
@alexandrasp alexandrasp force-pushed the add-support-for-xt-db-basis branch 2 times, most recently from 4514f39 to 2798e7f Compare August 9, 2023 05:53
@alexandrasp alexandrasp force-pushed the add-support-for-xt-db-basis branch from 2798e7f to 6b9ff37 Compare August 9, 2023 06:42

(defn db-basis->proto [db-basis-response]
Copy link
Owner Author

Choose a reason for hiding this comment

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

I think this change broke stuff, I would probably test it with duplication for now

Copy link
Owner Author

Choose a reason for hiding this comment

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

also, db-basis->proto and ->db-basis are different things and had different usages

@@ -19,12 +19,12 @@

(defn entity-tx [xtdb-node params]
(let [id (->id (:id-type params) (:entity-id params))
db-basis (->db-basis params)
db-basis (adapters.db/db-basis->proto params)
Copy link
Owner Author

Choose a reason for hiding this comment

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

it breaks here

Copy link
Owner Author

Choose a reason for hiding this comment

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

Keep the old one

Copy link
Owner Author

Choose a reason for hiding this comment

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

I would checkout src/gxtdb and start fresh

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok! I reverted ->db-basis, and now the tests are passing. Now, going to check src/gxtdb

Signed-off-by: Alexandra Pereira <[email protected]>
@alexandrasp alexandrasp marked this pull request as ready for review August 11, 2023 03:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for xt/db-basis
3 participants