Skip to content

fix: Implement sync, scheduler, resource encoding #44

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Aug 10, 2023
Merged

fix: Implement sync, scheduler, resource encoding #44

merged 5 commits into from
Aug 10, 2023

Conversation

erezrokah
Copy link
Member

Fixes #39

@erezrokah erezrokah requested a review from disq August 10, 2023 15:29
erezrokah
erezrokah commented Aug 10, 2023
View reviewed changes
src/scheduler/scheduler.ts
};

export const sync = async (client: ClientMeta, tables: Table[], stream: SyncStream, options: Options) => {
const resolveTable = async (
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the bulk of the change

erezrokah
erezrokah commented Aug 10, 2023
View reviewed changes
src/scheduler/scheduler.ts
}

stream.end();
await pMap(tables, (table) => resolveTable(logger, client, table, null, syncStream), { concurrency });
Copy link
Member Author

Choose a reason for hiding this comment

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

concurrency only enforced at the top level for now

disq
disq reviewed Aug 10, 2023
View reviewed changes
src/scalar/scalar.ts Outdated

export const newScalar = (dataType: DataType): Scalar<unknown> => {
if (DataType.isBool(dataType)) {
return new Bool(false);
Copy link
Member

Choose a reason for hiding this comment

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

Can't we pass undefined/null instead of zero values and init the scalars as invalid?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes we can, I'll update the classes

erezrokah
erezrokah commented Aug 10, 2023
View reviewed changes
src/schema/resource.ts
const { table } = resource;
const schema = toArrowSchema(table);
// TODO: Check if this can be simplified
let batch = new RecordBatch(schema, undefined);
Copy link
Member Author

Choose a reason for hiding this comment

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

There's a better way to do this I'm sure

disq
disq reviewed Aug 10, 2023
View reviewed changes
disq
disq approved these changes Aug 10, 2023
View reviewed changes
Copy link
Member

@disq disq left a comment

Choose a reason for hiding this comment

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

Nice

@erezrokah erezrokah added the automerge Add to automerge PRs once requirements are met label Aug 10, 2023
@kodiakhq kodiakhq bot merged commit 4a5f9e8 into main Aug 10, 2023
@kodiakhq kodiakhq bot deleted the fix/sync branch August 10, 2023 15:41
@erezrokah erezrokah mentioned this pull request Aug 10, 2023
Closed
1 task
@cq-bot cq-bot mentioned this pull request Aug 10, 2023
Merged
@erezrokah erezrokah mentioned this pull request Aug 10, 2023
Closed
kodiakhq bot pushed a commit that referenced this pull request Aug 16, 2023
@cq-bot
chore(main): Release v0.0.3 (#11)
ffa3979 
🤖 I have created a release *beep* *boop*
---


## [0.0.3](v0.0.2...v0.0.3) (2023-08-16)


### Features

* Add `addCQIDsColumns` util, split MemDB into files ([#51](#51)) ([dd71c60](dd71c60))
* Add arrow dependency ([#15](#15)) ([082213f](082213f))
* Add Bool scalar ([#17](#17)) ([80edcd5](80edcd5))
* Add CLI argument parsing ([#10](#10)) ([0d70b25](0d70b25))
* Add CQID and Parent CQID ([#53](#53)) ([fbb7c94](fbb7c94))
* Add initial sync, scheduler ([#37](#37)) ([9b41287](9b41287))
* Add JSON scalar ([#55](#55)) ([7e39769](7e39769))
* Add multiplexer, round-robin ([#48](#48)) ([00a842a](00a842a))
* Add String type ([#29](#29)) ([4c07a3d](4c07a3d))
* Add table types ([#32](#32)) ([c7b301d](c7b301d))
* Add UUID scalar ([#54](#54)) ([d8474ce](d8474ce))
* Add UUID, JSON types ([#56](#56)) ([45ab919](45ab919))
* Implement discovery service ([#19](#19)) ([3abe320](3abe320))
* Implement logger ([#21](#21)) ([43becf1](43becf1))
* List scalars ([#36](#36)) ([0eb6d62](0eb6d62))
* MemDB read ([#43](#43)) ([3429de0](3429de0)), closes [#41](#41)
* MemDB writes ([#42](#42)) ([8f21f52](8f21f52))
* More scalars ([#50](#50)) ([a9589b6](a9589b6))
* Scaffold plugin server ([#27](#27)) ([f0864eb](f0864eb))
* **scalars:** Timestamp, float64, int64 ([#35](#35)) ([7fc391c](7fc391c))
* Transform objects ([#34](#34)) ([c14f017](c14f017)), closes [#5](#5)
* Transformers ([#25](#25)) ([e96adb3](e96adb3))


### Bug Fixes

* @grpc/grpc-js dependency ([2adf06d](2adf06d))
* **deps:** Update dependency @cloudquery/plugin-pb-javascript to ^0.0.6 ([#14](#14)) ([92a87a1](92a87a1))
* Encode tables in migrate messages ([#38](#38)) ([f6413d2](f6413d2))
* Error handling, `null` values in scalars, proper exports ([#61](#61)) ([2b283d1](2b283d1))
* Extension types null values ([#58](#58)) ([e8bb0a0](e8bb0a0))
* Flatten tables in GetTables gRPC call ([#57](#57)) ([5a8f3c3](5a8f3c3))
* Implement sync, scheduler, resource encoding ([#44](#44)) ([4a5f9e8](4a5f9e8))
* Parent CQId resolver ([8de14cb](8de14cb))
* Properly encode uuid ([#59](#59)) ([72efa22](72efa22))
* Support only tcp network ([#23](#23)) ([e138e40](e138e40))
* Write gRPC call, use `for await` on write readble stream ([#52](#52)) ([773a0e5](773a0e5))

---
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
Reviewers

@disq disq disq approved these changes

Assignees
No one assigned
Labels
automerge Add to automerge PRs once requirements are met
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat: Make the sync gRPC call work
2 participants
@erezrokah @disq