Metadata info protobuf format support #8

NRHelmi · 2022-06-01T13:46:08Z

This PR adds support to the metadata info protobuf format.

cd RelationalAI.Examples
dotnet run ExecuteAsync --engine ENGINE --database DB --command "def output = 123"

This reverts commit 48dbdfa.

NHDaly · 2022-06-06T13:41:09Z

RelationalAI/TransactionAsyncResult.cs

            this.Metadata = metadata;
+            this.MetadataInfos = metadataInfos;


CC: @bachdavi - we were wondering if we should simply replace / delete the old json format, rather than supplying clients with both?

Or maybe we want one version where it's marked deprecated and then another released version after that that deletes it, so that customers can use the in-between version to more easily migrate?

denisgursky · 2022-08-02T15:25:40Z

RelationalAI.Protos/Proto/metadata.proto

@@ -0,0 +1,160 @@
+syntax = "proto3";


See: RelationalAI/rai-sdk-javascript#32 (comment)

larf311

Can you update the readme to include the fact that you need to install protobuf?

larf311 · 2022-08-02T19:49:31Z

This doesn't seem to work as the accept header doesn't seem to be getting passed through. I added some log statements to the example:

POST https://login.relationalai.com/oauth/token
Content-Type: application/json
GET https://azure.relationalai.com:443/transactions/206c49ca-bdc7-c7c8-8edd-ad49a211455b/metadata
Key = Authorization, Value = Bearer ...
Content-Type: application/json

NRHelmi · 2022-08-02T19:51:48Z

This doesn't seem to work as the accept header doesn't seem to be getting passed through. I added some log statements to the example:

POST https://login.relationalai.com/oauth/token
Content-Type: application/json
GET https://azure.relationalai.com:443/transactions/206c49ca-bdc7-c7c8-8edd-ad49a211455b/metadata
Key = Authorization, Value = Bearer eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6IlhaWVVsUTlhWm5sRGxZVFVfM05RTSJ9.eyJodHRwczovL3JlbGF0aW9uYWwuYWkvY2xhaW1zL2FjY291bnQiOiJyZWxhdGlvbmFsYWktdGVhbS1yZC11eCIsImlzcyI6Imh0dHBzOi8vbG9naW4ucmVsYXRpb25hbGFpLmNvbS8iLCJzdWIiOiIyOGVST3NsSU51WTBTZE15RFBOZ2xCbDJVRTAxR2txaEBjbGllbnRzIiwiYXVkIjoiaHR0cHM6Ly9henVyZS5yZWxhdGlvbmFsYWkuY29tIiwiaWF0IjoxNjU5NDY5NzEyLCJleHAiOjE2NTk0NzMzMTIsImF6cCI6IjI4ZVJPc2xJTnVZMFNkTXlEUE5nbEJsMlVFMDFHa3FoIiwic2NvcGUiOiJ1cGRhdGU6ZGF0YWJhc2UgbGlzdDpkYXRhYmFzZSBsaXN0OmNvbXB1dGUgcmVhZDpjb21wdXRlIGRlbGV0ZTpjb21wdXRlIGNyZWF0ZTpjb21wdXRlIHJ1bjp0cmFuc2FjdGlvbiIsImd0eSI6ImNsaWVudC1jcmVkZW50aWFscyJ9.RPXGxlTC1hnFRPym10vNBKWF9XskzKIKJH5UgUyxOrAzECD99tzZ_kWenBt-tbB5Zyr9a9S3Fb03eTyoDSMzxYyaHDPqZwuXc0ICxtVwAiyZPf8vQwJ8mwXCTnFqPMJinddgS3q57kpSyTlLEX3ch-GJu1ig_4sIz-XUol1lco46DfYgqdsM9pi8uB1kcTdPZHZMoF3lTPw7-VA96y5BJKE-L_3quPgDZatc23S8D1meBNG_kMliAWqLu-MElXDygmxDsg9RuKu8WgPLQr0s_BPsBk6JDsalpnmIUVDitYv56CWAeZ8mvPll0t_VCgP8-cTplkENP0iq0Jx_F6EceA
Content-Type: application/json

checking ... 👀

EDITED: I just double checked things seem to work my side

dotnet run GetTransactionMetadataInfo --id "206c49ca-bdc7-c7c8-8edd-ad49a211455b"
{ "relations": [ { "relationId": { "arguments": [ { "tag": "CONSTANT_TYPE", "constantType": { "relType": { "tag": "PRIMITIVE_TYPE", "primitiveType": "SYMBOL" }, "value": { "arguments": [ { "tag": "SYMBOL", "stringVal": "b3V0cHV0" } ] } } }, { "tag": "PRIMITIVE_TYPE", "primitiveType": "INT_64" } ] }, "fileName": "0.arrow" } ] }

btw I need to rename GetTransactionMetadataInfo to GetTransactionMetadata
not sure but probably you are not invoking the right code 🤔

larf311 · 2022-08-02T19:57:46Z

dotnet run GetTransactionMetadataInfo --id 206c49ca-bdc7-c7c8-8edd-ad49a211455b

larf311 · 2022-08-02T19:59:24Z

It seems to be removing the accept header on line 212 of Rest.cs. Gotta run. Will check back later.

larf311 · 2022-08-03T12:29:23Z

I tried again this morning and it's still not working for me.

vilterp · 2022-08-11T22:21:39Z

RelationalAI/TransactionAsyncResult.cs


    public class TransactionAsyncResult : Entity
    {
        public TransactionAsyncCompactResponse Transaction { get; set; }
        public List<ArrowRelation> Results { get; set; }
-        public List<TransactionAsyncMetadataResponse> Metadata { get; set; }
+        public MetadataInfo Metadata { get; set; }
        public List<object> Problems { get; set; }


Let's follow this up with a PR similar to RelationalAI/rai-sdk-java#16, introducing GotCompleteResult

larf311 · 2022-08-18T12:12:30Z

Should we have the schema.proto in every repo? Seems like it should be centralized.

larf311

There are lots of warnings about closing braces on the Github diff. Should we address these? If not, where are they coming from?

NRHelmi · 2022-08-18T15:33:15Z

Should we have the schema.proto in every repo? Seems like it should be centralized.

I remember we discussed this with the team at some point and It seems like we need to live with the local dependency on proto specificaiton until we figure out how to centralize it 🙏

NRHelmi · 2022-08-18T15:36:52Z

There are lots of warnings about closing braces on the Github diff. Should we address these? If not, where are they coming from?

I don't really remember setting up something for csharp linting but indeed we need to do so
let me check if I can setup something 🙏

NHDaly · 2022-08-18T17:41:33Z

Should we have the schema.proto in every repo? Seems like it should be centralized.

@larf311, agreed. There's an issue for it here: https://github.com/RelationalAI/rai-sdk-issues/issues/62

EDIT: Yeah, what Helmi said. 👍

NRHelmi · 2022-08-19T14:52:14Z

if no other updates are required, I'll merge by the end of the day thank you

NRHelmi added 5 commits June 1, 2022 14:10

adding support to metadata info protobuf format

48dbdfa

Revert "adding support to metadata info protobuf format"

322f5cd

This reverts commit 48dbdfa.

adding support to metadata info protobuf format

1f8c2af

cleanup

7aece42

proto update

7736da4

NHDaly requested a review from bachdavi June 6, 2022 13:38

NHDaly reviewed Jun 6, 2022

View reviewed changes

NRHelmi added 2 commits July 8, 2022 14:54

sync with main

8e72af3

Added get transaction protobuf metadata

39671ce

NRHelmi marked this pull request as ready for review July 10, 2022 15:59

NRHelmi requested a review from vilterp July 10, 2022 15:59

NRHelmi requested review from larf311 and denisgursky August 1, 2022 15:37

NRHelmi added 6 commits August 2, 2022 00:10

formatting

2787e06

cleanup

67cd7f7

fix

c3f5b6e

fix metadata.pb path

6ffa664

fixing relation id in ExecAsync test

cc9cf7e

remove output type from Proto project

e40b36d

denisgursky reviewed Aug 2, 2022

View reviewed changes

remove internal server proto file

777560e

larf311 reviewed Aug 2, 2022

View reviewed changes

support also osx

843331b

GetTransactionMetadataInfo to GetTransactionMetadata

c962b9b

adding metadata.pb.txt

8fcd9a3

updated changelog && bump up version

e9eb94a

vilterp reviewed Aug 11, 2022

View reviewed changes

NRHelmi added 2 commits August 16, 2022 18:33

sync with main

5d7c492

fix metadata proto test issue

c7013e9

larf311 reviewed Aug 18, 2022

View reviewed changes

NRHelmi added 6 commits August 18, 2022 17:14

code formatting using dotnet format

bf38057

new lines added

d86d9aa

linter check added

78d2114

simulate bad written code

3110eae

fix dotner command

5acda8e

fix lint issue

a029ef5

NRHelmi added 4 commits August 19, 2022 14:30

apply styling

066c440

sync with main

fad0ad5

fix linting

d8fe19c

fix test

32a7203

NRHelmi merged commit 490cbdf into main Aug 21, 2022

NRHelmi deleted the hnr-metadata-info-protobuf-format branch August 21, 2022 00:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Metadata info protobuf format support #8

Metadata info protobuf format support #8

NRHelmi commented Jun 1, 2022 •

edited

Loading

NHDaly Jun 6, 2022

denisgursky Aug 2, 2022

larf311 left a comment

larf311 commented Aug 2, 2022 •

edited

Loading

NRHelmi commented Aug 2, 2022 •

edited

Loading

larf311 commented Aug 2, 2022

larf311 commented Aug 2, 2022 •

edited

Loading

larf311 commented Aug 3, 2022

vilterp Aug 11, 2022

larf311 commented Aug 18, 2022

larf311 left a comment

NRHelmi commented Aug 18, 2022

NRHelmi commented Aug 18, 2022

NHDaly commented Aug 18, 2022 •

edited

Loading

NRHelmi commented Aug 19, 2022

		this.Metadata = metadata;
		this.MetadataInfos = metadataInfos;

Metadata info protobuf format support #8

Metadata info protobuf format support #8

Conversation

NRHelmi commented Jun 1, 2022 • edited Loading

NHDaly Jun 6, 2022

Choose a reason for hiding this comment

denisgursky Aug 2, 2022

Choose a reason for hiding this comment

larf311 left a comment

Choose a reason for hiding this comment

larf311 commented Aug 2, 2022 • edited Loading

NRHelmi commented Aug 2, 2022 • edited Loading

larf311 commented Aug 2, 2022

larf311 commented Aug 2, 2022 • edited Loading

larf311 commented Aug 3, 2022

vilterp Aug 11, 2022

Choose a reason for hiding this comment

larf311 commented Aug 18, 2022

larf311 left a comment

Choose a reason for hiding this comment

NRHelmi commented Aug 18, 2022

NRHelmi commented Aug 18, 2022

NHDaly commented Aug 18, 2022 • edited Loading

NRHelmi commented Aug 19, 2022

NRHelmi commented Jun 1, 2022 •

edited

Loading

larf311 commented Aug 2, 2022 •

edited

Loading

NRHelmi commented Aug 2, 2022 •

edited

Loading

larf311 commented Aug 2, 2022 •

edited

Loading

NHDaly commented Aug 18, 2022 •

edited

Loading