Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implement getTables #71

Merged
merged 5 commits into from
Aug 21, 2023
Merged

feat: Implement getTables #71

merged 5 commits into from
Aug 21, 2023

Conversation

erezrokah
Copy link
Member

@erezrokah erezrokah commented Aug 18, 2023

Still doesn't work and I need to handle the options like include, skip, etc.

Got the gRPC call to work.

Had to:

  1. Add org.apache.arrow:arrow-memory-core as implementation dependency
  2. Add org.apache.arrow:arrow-memory-nett as runtime only dependency (see https://mvnrepository.com/artifact/org.apache.arrow/arrow-memory-netty/12.0.1)
  3. Add another gradle task so the runtime class is loaded properly. We'll need see how this works with a published version of the SDK.

@erezrokah erezrokah marked this pull request as ready for review August 21, 2023 12:59
testImplementation('nl.jqno.equalsverifier:equalsverifier:3.15')
testRuntimeOnly('org.junit.jupiter:junit-jupiter-engine:5.10.0')

testImplementation 'org.assertj:assertj-core:3.24.2'

runtimeOnly "org.apache.arrow:arrow-memory-netty:12.0.1"
Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -83,3 +89,11 @@ publishing {
}
}
}

task runMemDBServe(type: JavaExec) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this so I can debug the MemDB plugin via VSCode without having to manually configure class paths

}

@Override
public void init(io.cloudquery.plugin.v3.Init.Request request,
Copy link
Member Author

Choose a reason for hiding this comment

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

To be implemented later

List<Table> tables = plugin.tables();
List<ByteString> byteStrings = new ArrayList<>();
for (Table table : tables) {
try (BufferAllocator bufferAllocator = new RootAllocator()) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@Override
public void sync(io.cloudquery.plugin.v3.Sync.Request request,
StreamObserver<io.cloudquery.plugin.v3.Sync.Response> responseObserver) {
plugin.sync();
Copy link
Member Author

Choose a reason for hiding this comment

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

To be implemented later

@Override
public void read(io.cloudquery.plugin.v3.Read.Request request,
StreamObserver<io.cloudquery.plugin.v3.Read.Response> responseObserver) {
plugin.read();
Copy link
Member Author

Choose a reason for hiding this comment

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

To be implemented later


@Override
public StreamObserver<Write.Request> write(StreamObserver<Write.Response> responseObserver) {
plugin.write();
Copy link
Member Author

Choose a reason for hiding this comment

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

To be implemented later

@Override
public void close(io.cloudquery.plugin.v3.Close.Request request,
StreamObserver<io.cloudquery.plugin.v3.Close.Response> responseObserver) {
plugin.close();
Copy link
Member Author

Choose a reason for hiding this comment

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

To be implemented later

this.item = item;
this.parent = parent;
this.table = table != null ? table : Table.builder().build();
this.table = table;
Copy link
Member Author

Choose a reason for hiding this comment

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

Forces a table when creating a resource

// Configure open telemetry
// Configure test listener
// Configure gRPC server
try (LoggerContext context = this.initLogger()) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Some cleanup to avoid the need for a finally block

Copy link
Collaborator

@mnorbury mnorbury left a comment

Choose a reason for hiding this comment

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

Looks good to me, I had a few comments.

@@ -34,6 +38,7 @@ dependencies {
implementation "io.grpc:grpc-services:1.57.1"
implementation "io.grpc:grpc-testing:1.57.1"
implementation "io.cloudquery:plugin-pb-java:0.0.5"
implementation "org.apache.arrow:arrow-memory-core:12.0.1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unrelated to this PR, but maybe we should introduce version variables since we have multiple imports for several packages now.

Comment on lines +66 to +67
writer.start();
writer.end();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I was expecting a call to a write method here, or is the call to end writing data?

Copy link
Member Author

@erezrokah erezrokah Aug 21, 2023

Choose a reason for hiding this comment

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

Good question, start writes the schema which is what we want here. We don't need to write any records (this will happen during sync) so there's no write.
https://github.com/apache/arrow/blob/6357c9f2419d5b0717e62adc8233c649e10de34b/java/vector/src/main/java/org/apache/arrow/vector/ipc/ArrowWriter.java#L183

@erezrokah erezrokah added the automerge Add to automerge PRs once requirements are met label Aug 21, 2023
Copy link
Collaborator

@mnorbury mnorbury left a comment

Choose a reason for hiding this comment

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

🚀

@kodiakhq kodiakhq bot merged commit 085c51f into main Aug 21, 2023
4 checks passed
@kodiakhq kodiakhq bot deleted the feat/tables_grpc branch August 21, 2023 13:44
kodiakhq bot pushed a commit that referenced this pull request Aug 24, 2023
🤖 I have created a release *beep* *boop*
---


## 0.0.1 (2023-08-24)


### Features

* `io.cloudquery.scalar.Binary` implementation ([#20](#20)) ([b9b73d1](b9b73d1))
* `io.cloudquery.scalar.Bool` ([#27](#27)) ([2a91c92](2a91c92)), closes [#26](#26)
* adding basic support for tables ([#19](#19)) ([22b2350](22b2350))
* adding JSON scalar ([#82](#82)) ([fc92542](fc92542)), closes [#63](#63)
* adding Table filterDFS functionaility ([#21](#21)) ([02d8515](02d8515))
* Date scalars ([#36](#36)) ([adc6ba2](adc6ba2)), closes [#34](#34)
* Duration scalar ([#42](#42)) ([7529438](7529438)), closes [#39](#39)
* Encode resources with data ([#88](#88)) ([2c7060f](2c7060f))
* Generics in scalars ([#56](#56)) ([bc7d6e3](bc7d6e3))
* Implement `getTables` ([#71](#71)) ([085c51f](085c51f))
* Implement concurrency and relations resolving ([#91](#91)) ([0a470b7](0a470b7))
* Init logger, add initial MemDB plugin ([#70](#70)) ([20ebb42](20ebb42))
* int/uint/float/string scalars ([#59](#59)) ([39ec6e6](39ec6e6)), closes [#53](#53) [#54](#54) [#58](#58) [#60](#60)
* Resolve CQId, add CQIds to MemDB plugin ([#95](#95)) ([9d7f1bd](9d7f1bd))
* Scalar Timestamp ([#46](#46)) ([4220e92](4220e92)), closes [#44](#44)
* **sync:** Initial insert message support ([#81](#81)) ([bd729bb](bd729bb))
* **sync:** Send migrate messages ([#79](#79)) ([dd2c1a5](dd2c1a5))


### Bug Fixes

* Add `jackson-annotations` to `build.gradle` ([#83](#83)) ([ead7dd9](ead7dd9))
* **deps:** Update dependency com.google.guava:guava to v32 ([#15](#15)) ([ce8028b](ce8028b))
* **deps:** Update dependency io.grpc:grpc-protobuf to v1.57.1 ([#10](#10)) ([bcfa29c](bcfa29c))
* **deps:** Update dependency io.grpc:grpc-services to v1.57.1 ([#11](#11)) ([71c2ea1](71c2ea1))
* **deps:** Update dependency io.grpc:grpc-stub to v1.57.1 ([#12](#12)) ([c65e5d6](c65e5d6))
* **deps:** Update dependency io.grpc:grpc-testing to v1.57.1 ([#13](#13)) ([a7b1fa6](a7b1fa6))
* **deps:** Update plugin org.gradle.toolchains.foojay-resolver-convention to v0.6.0 ([#14](#14)) ([443990c](443990c))
* Flatten tables in getTables gRPC call ([#80](#80)) ([8c9872a](8c9872a))
* Pass options to tables method ([#78](#78)) ([4b77a2f](4b77a2f))


### Miscellaneous Chores

* Release 0.0.1 ([e169dbc](e169dbc))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Add to automerge PRs once requirements are met
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants