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

Component metadata and ApiDefinition data serde #629

Merged

justcoon merged 32 commits into main from component_serde

Jul 12, 2024

Contributor

justcoon commented Jun 29, 2024 •

edited

Loading

related to: #498

added namespace to component, metadata using proto for serialization, with version
repository and service moved to golem-component-service-base
fixed naming for FunctionParameter, FunctionResult - tpe -> typ
api definition serialization changed from bincode to proto, proto for golem-rib

senia-psm suggested changes

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

golem-component-service-base/src/repo/component.rs Show resolved Hide resolved

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

golem-component-service-base/src/repo/component.rs Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/tests/db/migration/sqlite/001__init.sql Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/tests/db/migration/sqlite/001__init.sql Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/tests/db/migration/sqlite/001__init.sql Outdated Show resolved Hide resolved

justcoon force-pushed the component_serde branch from 9a0b9d0 to c5b78aa Compare

July 2, 2024 16:20

senia-psm reviewed

View reviewed changes

golem-component-service/db/migration/postgres/001__init.sql Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service/db/migration/postgres/001__init.sql Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service/db/migration/postgres/001__init.sql Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service/db/migration/sqlite/001__init.sql Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/tests/db/migration/postgres/001__init.sql Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/tests/db/migration/postgres/001__init.sql Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/tests/db/migration/postgres/001__init.sql Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

senia-psm reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

afsalthaj reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

afsalthaj reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Outdated Show resolved Hide resolved

afsalthaj reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs Show resolved Hide resolved

afsalthaj reviewed

View reviewed changes

golem-component-service-base/src/repo/component.rs

+                          sqlx::query(
+                              r#"
+                                INSERT INTO components
+                                  (namespace, component_id, name)

Contributor

afsalthaj Jul 3, 2024

if namespace and component_id are part of key, then you can avoid extra checks probably

Contributor

afsalthaj Jul 3, 2024

and Unique name for each namespace

Contributor

senia-psm Jul 3, 2024 •

edited

Loading

@afsalthaj there is already a unique index on (namespace, name). And component_id is a PK.

We are creating a new version here (in component_version). And for each new version we are creating component record first. It might exists already (if there are other versions) or not (if we are creating the first versions) - both options are not an error.

The only problem here is if we are trying to create a new version for some component id with different namespace or name comparing to the previous version.

justcoon added 11 commits

July 5, 2024 09:32


          component-service logs, namespace check

71a917e


          component-service create with id

1452b27


          naming fix - tpe -> typ

ddc25e9


          components and component_versions tables, repo update

2a8dacf


          component-service get_id_by_name, get_namespace

0e21ac2


          component-repo queries with specific column names

e6cdf75


          component-repo upsert -> create

4ccf0f3


          component-repo sql format, tests

345ba22


          remove unnecessary fields from component_versions table

497df2d


          components and api_definition - proto serde

da66f39


          merge fixes

c0e8a1e

justcoon force-pushed the component_serde branch from dce96c9 to c0e8a1e Compare

July 5, 2024 07:41

Contributor

afsalthaj commented Jul 5, 2024

So I can see response mapping being a string in proto, while previously bincode - encode decode directly serde'd.
While there is a serde involved, it is definitely faster than combine-parsing Expr again!

https://zivergeteam.slack.com/archives/C057S2E4XT5/p1720164992876289?thread_ts=1720164896.439539&cid=C057S2E4XT5


          WorkerBinding proto - worker_id -> worker_name

3d3f8f9

afsalthaj dismissed their stale review

July 5, 2024 09:45

Changes to be done after we moved to profobuf

justcoon added 9 commits

July 5, 2024 21:22


          golem-rib proto

6665f8e


          WorkerBinding proto - golem.rib.Expr

b1c5599


          tests

37dd564


          component download - namespace check

ac5baec


          component with namespace

de45178


          repo - select by id without namespace

803db08


          Merge branch 'refs/heads/main' into component_serde

d64fb05

# Conflicts:
#	golem-worker-service/src/grpcapi/api_definition.rs
#	golem-worker-service/src/grpcapi/worker.rs


          component repo - delete

8ed30a6


          logs, tests

eb396e0

senia-psm reviewed

View reviewed changes

golem-component-service-base/tests/services_tests.rs Outdated Show resolved Hide resolved

senia-psm approved these changes

View reviewed changes

Contributor

senia-psm left a comment

Unused index and no name validation on create. Otherwise perfect.

justcoon added 2 commits

July 10, 2024 14:08


          fixes

a75cc52


          Merge branch 'refs/heads/main' into component_serde

4d746e2

justcoon merged commit e68f3f5 into main

14 checks passed

justcoon deleted the component_serde branch

July 12, 2024 17:06

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet