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.

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

feat: Migrate plugin to github.com/cloudquery/plugin-sdk/v2 #19

Closed
candiduslynx opened this issue Apr 17, 2023 · 13 comments
Closed

feat: Migrate plugin to github.com/cloudquery/plugin-sdk/v2 #19

candiduslynx opened this issue Apr 17, 2023 · 13 comments

Comments

@candiduslynx
Copy link
Contributor

Hello!
We've recently updated plugin-sdk to v2 (includes migration to Apache Arrow for the destination side).

Currently, no major updates to the source side were made.

Here's an easy step-by-step workflow that will allow you to migrate the code easily & efficiently:

  1. Install gomajor
  2. gomajor get github.com/cloudquery/plugin-sdk/v2
  3. add replace for github.com/apache/arrow/go/v12 (see github.com/cloudquery/plugin-sdk/v2 go.mod)
  4. go mod tidy && go get -t -d ./... && go mod tidy && go mod vendor
  5. make gen lint test (if the appropriate targets exist)
@candiduslynx candiduslynx mentioned this issue Apr 17, 2023
Closed
73 tasks
koltyakov added a commit that referenced this issue Apr 17, 2023
@koltyakov
feat(sdk): Migration to v2, #19
98a3384
@koltyakov
Copy link
Owner

Thank you @candiduslynx!
Migrated to plugin-sdk to v2.
Tested, no issues found.

@koltyakov
Copy link
Owner

@candiduslynx, I'm testing with updated destinations plugins such as SQLite v2.0.0 or PostgreSQL v4.0.0 and getting the error:

is missing column _cq_id. please consider upgrading source plugin

With older version of destination plugins it works.

Could I miss something while updating to plugin-sdk/v2?

@koltyakov koltyakov reopened this Apr 18, 2023
@candiduslynx
Copy link
Contributor Author

@koltyakov try with CLI 2.5.3

@koltyakov
Copy link
Owner

@candiduslynx I was already using the latest (cloudquery/tap/cloudquery 2.5.3 already installed).
It is something else.

@koltyakov
Copy link
Owner

koltyakov commented Apr 18, 2023 •
edited
Loading

It's seems not on my end. I tried the following as an experiment:

  • took a configuration of postgre -> sqlite which worked before,
  • updated potgresql source to v1.0.8 which uses plugin-sdk v2,
  • updates sqlite destination to v2.0.0 which also migrated to plugin-sdk v2,
  • and also receiving the same error:
Starting migration with 1 tables for: coop-prod ([email protected]) -> [coop-local ([email protected])]
Error: failed to sync v1 source coop-prod: failed to migrate source coop-prod ([email protected]) 
  on destination coop-local ([email protected]) : failed to call Migrate: rpc error: 
  code = Unknown desc = table artist is missing column _cq_id. please consider upgrading source plugin

Tried not only SQLite destination but also PostgreSQL with v2, same issue.

The error could be connected with incremental mode and throughs from plugins/destination/plugin.go

@yevgenypats maybe you know show could check this on your end, seems that it's global thing effected with plugin-sdk v2.

@yevgenypats
Copy link
Contributor

It's seems not on my end. I tried the following as an experiment:

  • took a configuration of postgre -> sqlite which worked before,
  • updated potgresql source to v1.0.8 which uses plugin-sdk v2,
  • updates sqlite destination to v2.0.0 which also migrated to plugin-sdk v2,
  • and also receiving the same error:
Starting migration with 1 tables for: coop-prod ([email protected]) -> [coop-local ([email protected])]
Error: failed to sync v1 source coop-prod: failed to migrate source coop-prod ([email protected]) 
  on destination coop-local ([email protected]) : failed to call Migrate: rpc error: 
  code = Unknown desc = table artist is missing column _cq_id. please consider upgrading source plugin

Tried not only SQLite destination but also PostgreSQL with v2, same issue.

The error could be connected with incremental mode and throughs from plugins/destination/plugin.go

@yevgenypats maybe you know show could check this on your end, seems that it's global thing effected with plugin-sdk v2.

Taking a look

@erezrokah
Copy link

Hi @koltyakov, thanks for reporting this issue. Can you try with version v2.0.2 of the SQLIte plugin? The issue should be fixed in that version

@andrew-koltyakov-wmg
Copy link

Thank you @yevgenypats, @erezrokah!

Checking with sqlite v2.0.2:

Starting migration with 1 tables for: sharepoint (local@../bin/cq-source-sharepoint) -> [sqlite (v2.0.2)]
panic: unknown type

goroutine 37 [running]:
github.com/cloudquery/cloudquery/plugins/destination/sqlite/client.(*Client).sqliteTypeToArrowType(...)
        /__w/cloudquery/cloudquery/plugins/destination/sqlite/client/types.go:54
github.com/cloudquery/cloudquery/plugins/destination/sqlite/client.(*Client).sqliteTables(0x14000370da8?, {0x140001a4418, 0x1, 0x10?})
        /__w/cloudquery/cloudquery/plugins/destination/sqlite/client/migrate.go:52 +0x7d0
github.com/cloudquery/cloudquery/plugins/destination/sqlite/client.(*Client).Migrate(0x140000d24b0, {0x1?, 0x0?}, {0x140001a4408?, 0x140001a4410?, 0x140001a4408?})
        /__w/cloudquery/cloudquery/plugins/destination/sqlite/client/migrate.go:144 +0x58
github.com/cloudquery/plugin-sdk/v2/plugins/destination.(*Plugin).Migrate(0x140001f7080, {0x105aa0e40, 0x1400039a960}, {0x140001a4408, 0x1, 0x1})
        /go/pkg/mod/github.com/cloudquery/plugin-sdk/[email protected]/plugins/destination/plugin.go:191 +0x78
github.com/cloudquery/plugin-sdk/v2/internal/servers/destination/v0.(*Server).Migrate(0x140003a0000, {0x105aa0e40, 0x1400039a960}, 0x140001a6ea0)
        /go/pkg/mod/github.com/cloudquery/plugin-sdk/[email protected]/internal/servers/destination/v0/destinations.go:64 +0x12c
github.com/cloudquery/plugin-sdk/v2/internal/pb/destination/v0._Destination_Migrate_Handler.func1({0x105aa0e40, 0x1400039a960}, {0x105a293e0?, 0x140001a6ea0})
        /go/pkg/mod/github.com/cloudquery/plugin-sdk/[email protected]/internal/pb/destination/v0/destination_grpc.pb.go:361 +0x74
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors.UnaryServerInterceptor.func1({0x105aa0e40, 0x1400039a5d0}, {0x105a293e0, 0x140001a6ea0}, 0x140000809f8?, 0x140001a2be8)
        /go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/[email protected]/interceptors/server.go:22 +0x19c
github.com/cloudquery/plugin-sdk/v2/internal/pb/destination/v0._Destination_Migrate_Handler({0x105a47da0?, 0x140003a0000}, {0x105aa0e40, 0x1400039a5d0}, 0x140003925b0, 0x140001a2900)
        /go/pkg/mod/github.com/cloudquery/plugin-sdk/[email protected]/internal/pb/destination/v0/destination_grpc.pb.go:363 +0x134
google.golang.org/grpc.(*Server).processUnaryRPC(0x140003681e0, {0x105aa5b18, 0x140004141a0}, 0x14000390120, 0x1400039a1e0, 0x10608e2a0, 0x0)
        /go/pkg/mod/google.golang.org/[email protected]/server.go:1345 +0xc50
google.golang.org/grpc.(*Server).handleStream(0x140003681e0, {0x105aa5b18, 0x140004141a0}, 0x14000390120, 0x0)
        /go/pkg/mod/google.golang.org/[email protected]/server.go:1722 +0x840
google.golang.org/grpc.(*Server).serveStreams.func1.2()
        /go/pkg/mod/google.golang.org/[email protected]/server.go:966 +0x84
created by google.golang.org/grpc.(*Server).serveStreams.func1
        /go/pkg/mod/google.golang.org/[email protected]/server.go:964 +0x294
failed to terminate destination client:  destination plugin process failed with exit status 2
Error: failed to sync v1 source sharepoint: failed to migrate source sharepoint (local@../bin/cq-source-sharepoint) on destination sqlite (v2.0.2) : failed to call Migrate: rpc error: code = Unavailable desc = error reading from server: EOF
make: *** [sync-sqlite] Error 1

when dealing with a blank new db file:

cloudquery sync sharepoint.yml sqlite.yml
Loading spec(s) from sharepoint.yml, sqlite.yml
Starting migration with 9 tables for: sharepoint (local@../bin/cq-source-sharepoint) -> [sqlite (v2.0.2)]
Migration completed successfully.
Starting sync for: sharepoint (local@../bin/cq-source-sharepoint) -> [sqlite (v2.0.2)]
Sync completed successfully. Resources: 2013, Errors: 0, Panics: 0, Time: 11s

# Secondary run also works

it looks better.

Yet panics for existing data/structure from the pre v2 times.

@erezrokah erezrokah mentioned this issue Apr 19, 2023
Merged
@erezrokah
Copy link

Thanks for reporting @andrew-koltyakov-wmg, I believe cloudquery/cloudquery#10202 should fix it

kodiakhq bot pushed a commit to cloudquery/cloudquery that referenced this issue Apr 19, 2023
@erezrokah
fix: Add timestamp type (#10202)
5bbf8dd 

#### Summary

Related to koltyakov/cq-source-sharepoint#19 (comment).

SQLite doesn't have an official `timestamp` type but the Go lib takes care of it, see mattn/go-sqlite3#748.

We had this type in `v1`, see https://github.com/cloudquery/cloudquery/blob/plugins-destination-sqlite-v1.3.6/plugins/destination/sqlite/client/types.go#L58

<!--
@erezrokah
Copy link

Hi @andrew-koltyakov-wmg, sorry for all the trouble, do you mind trying with v2.0.3?

@koltyakov
Copy link
Owner

koltyakov commented Apr 19, 2023 •
edited
Loading

Hi @erezrokah, with v2.0.3 it works as expected! Thank you so much guys!

To check I recreated the scenario.

  • Dropped the db
  • Migrated with pre v2
  • Checked I'm getting _cq_id and exception with v2.0.0..v2.0.2
  • Checked that v2.0.3 doesn't crash but suggest the migration flow

Perfect!

Loading spec(s) from sharepoint.yml, sqlite.yml
Starting migration with 9 tables for: sharepoint (local@../bin/cq-source-sharepoint) -> [sqlite (v2.0.3)]
Error: failed to sync v1 source sharepoint: failed to migrate source sharepoint (local@../bin/cq-source-sharepoint) on destination sqlite (v2.0.3) : failed to call Migrate: rpc error: code = Unknown desc = tables sharepoint_mmd_department,sharepoint_user,sharepoint_search_media with changes [[{update tagging tagging: type=bool, nullable tagging: type=int64, nullable} {update deprecated deprecated: type=bool, nullable deprecated: type=int64, nullable} {update pinned pinned: type=bool, nullable pinned: type=int64, nullable} {update reused reused: type=bool, nullable reused: type=int64, nullable} {update root root: type=bool, nullable root: type=int64, nullable} {update source source: type=bool, nullable source: type=int64, nullable}] [{update issiteadmin issiteadmin: type=bool, nullable issiteadmin: type=int64, nullable} {update deleted deleted: type=bool, nullable deleted: type=int64, nullable}] [{update is_document is_document: type=bool, nullable is_document: type=int64, nullable}]] require force migration. use 'migrate_mode: forced'
make: *** [sync-sqlite] Error 1

@erezrokah
Copy link

Thanks for confirming @koltyakov! I think the forced migration is expected since we switched to using bool type for booleans instead of int. SQLite doesn't support that natively by the Go library we use handles it https://github.com/mattn/go-sqlite3/blob/edc3bb69551dcfff02651f083b21f3366ea2f5ab/doc.go#L21

@koltyakov
Copy link
Owner

@erezrokah yep, I didn't mentioned explicitly. For me that was expected and correct behavior. Once again thank you guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@candiduslynx @koltyakov @yevgenypats @erezrokah @andrew-koltyakov-wmg