feat(postgresql): Add support for overwrite-delete-stale by yevgenypats · Pull Request #2220 · cloudquery/cloudquery

yevgenypats · 2022-10-02T22:06:49Z

Summary

This adds the following:

Support for delete stale via feat!: Add overwrite-delete-stale mode for destination plugins plugin-sdk#224
numerous bugfixes
much needed tests for the postgresql destination plugin and split to number of files as it grew alot already.
report status via normal counts and not via logs so behaviour is consistent between debug/release and in general log parsing can be quite fragile.

hermanschaaf

LGTM 👍 I left some comments, nothing major stands out though. Some changes will be needed after cloudquery/plugin-sdk#224 is updated / merged.

cli/go.mod

hermanschaaf · 2022-10-03T09:54:28Z

plugins/destination/postgresql/client/deletestale_test.go

+		t.Fatalf("got %d, expected json_agg to return list with one entry", len(results))
+	}
+	// TODO: figure normal pgx<->go time normal conversion
+	syncTime := time.Now().UTC().Add(-24*30*12 * time.Hour)


Why is the time being added here so specific (negative ~12 months)?

plugins/destination/postgresql/client/drop_test.go

hermanschaaf · 2022-10-03T09:55:44Z

plugins/destination/postgresql/client/drop_test.go

+		t.Fatalf("failed to migrate tables: %v", err)
+	}
+
+	isExist, err := c.isTableExistSQL(ctx, testTable.Name)


Nit: I'd probably rename isExist to exists or doesExist

plugins/destination/postgresql/client/migrate_test.go

hermanschaaf · 2022-10-03T09:58:34Z

plugins/destination/postgresql/client/types.go

+	"github.com/cloudquery/plugin-sdk/schema"
+)
+
+func SchemaTypeToPg(t schema.ValueType) string {


Just a note: this move will conflict with the changes in #2167

hermanschaaf · 2022-10-03T10:00:01Z

plugins/destination/postgresql/client/write_test.go

+	"github.com/jackc/pgx/v4/pgxpool"
+)
+
+func TestWriteOverwrite(t *testing.T) {


This should probably be named TestWriteOverwriteDeleteStale

hermanschaaf · 2022-10-03T10:01:24Z

plugins/destination/postgresql/client/write_test.go

+			t.Fatalf("failed to parse _cq_id: %v", err)
+		}
+		if diff := cmp.Diff(result, expectedData[cqId.String()]); diff != "" {
+			t.Fatal(diff)


Let's add a string to this t.Fatal to describe what we were comparing, to add some context for the reader

add something but not much to add as it's just compares it with what we interested before so basically "unexpected results" as the diff prints the context already (what we expected with what we received )

plugins/destination/postgresql/go.mod

plugins/source/gcp/go.mod

erezrokah

Added a few minor comments pending the live sync.

Also, what happens if the write operation here fails?

Doesn't this mean existing data of failed writes/syncs will get deleted too? TLDR: How is the 3rd bullet Resources should only be deleted if the list was successful in #2168 (comment) is addressed?

erezrokah · 2022-10-03T08:25:01Z

cli/go.mod

+replace github.com/cloudquery/plugin-sdk => ../../plugin-sdk
+


Suggested change

replace github.com/cloudquery/plugin-sdk => ../../plugin-sdk

plugins/destination/postgresql/client/deletestale.go

erezrokah · 2022-10-03T09:13:28Z

plugins/source/gcp/go.mod

+replace github.com/cloudquery/plugin-sdk => ../../../../plugin-sdk
+


Suggested change

replace github.com/cloudquery/plugin-sdk => ../../../../plugin-sdk

erezrokah · 2022-10-03T09:20:14Z

plugins/destination/postgresql/main.go

 )

+var (
+	Version = "Development"


The convention we use is to put Version under resources/plugin, see

cloudquery/plugins/source/gcp/resources/plugin/plugin.go

Line 9 in bbc14cc

Version = "development"

cloudquery/plugins/.goreleaser.yaml

Line 12 in bbc14cc

- -s -w -X github.com/cloudquery/cloudquery/plugins/{{ .Var.component }}/resources/plugin.Version={{.Version}}

We can change the convention but should ensure all plugins use the same one (or have a different config for destination plugins)

I see. ok I'll move it that folder for now even though for destination it's a bit weird but not a big deal.

maybe we can actually do one for destinations ? as we don't really have resources there so that's a bit weird? maybe later we can come up with a convention that works for both?

Yeah, I think it's not a blocker for now (the version won't get embedded, but it won't fail the release)

cli/go.mod

yevgenypats · 2022-10-03T11:28:07Z

Added a few minor comments pending the live sync.

Also, what happens if the write operation here fails?

Doesn't this mean existing data of failed writes/syncs will get deleted too? TLDR: How is the 3rd bullet Resources should only be deleted if the list was successful in #2168 (comment) is addressed?

Exactly. There is no possible way to know what succeeded what not, because of which errors and so on. thus if you use this option then you can expect issue. the other suggested option is to write cron that will delete stale data older than something. Let's say you run sync every day and cron delete every two days.

CloudQuery SDK cannot detect what was delete on a pull based method, it can only be done if we integrate with something like cloudtrail which send events about deleted resources.

checking if operation list was successful is just one of infinite amount of edge-cases. What if list was successful but then all get failed? list was partially successful? what if we had dns errors in the middle, what if permission changed and we go returned a different number of resources and now deleted resources that actually exist? or permissions changed in such way that it just returned an empty list (which is very common in some APIs).

This introduce number of things: - Major: Adds `overwrite-deletestale` mode - Major: make destination plugin API consistent with source plugin api - Minor: removes dep on snapshot testing library as we want to avoid this and use standard comparison and/or google diff library (see one of many articles - https://medium.com/@sapegin/whats-wrong-with-snapshot-tests-37fbe20dfe8e). Also had extensive experience with snapshot testing in React so def agree with the article/s Related postgresql and CLI support - cloudquery/cloudquery#2220 Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>

Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>

erezrokah

Pending an SDK update 🚀

yevgenypats · 2022-10-03T15:50:27Z

Pending an SDK update 🚀

Updated. trying to fix tests.

yevgenypats · 2022-10-03T15:53:18Z

@erezrokah any idea what those golints issues mean ?

yevgenypats · 2022-10-03T17:56:03Z

ok this almost works. I'll merge this and follow-up with a quick fix.
Because I've added additional method to the protocol the CLI fails. In general what I had to do is first release a new test plugin with the new protocol and then release a new CLI.

🤖 I have created a release *beep* *boop* --- ## [1.2.0](plugins-source-test-v1.1.6...plugins-source-test-v1.2.0) (2022-10-03) ### Features * **postgresql:** Add support for overwrite-delete-stale ([#2220](#2220)) ([efdd136](efdd136)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- ## [1.2.0](plugins-destination-test-v1.1.0...plugins-destination-test-v1.2.0) (2022-10-03) ### Features * **postgresql:** Add support for overwrite-delete-stale ([#2220](#2220)) ([efdd136](efdd136)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- ## [0.11.0-pre.0](plugins-source-gcp-v0.10.8-pre.0...plugins-source-gcp-v0.11.0-pre.0) (2022-10-04) ### Features * **postgresql:** Add support for overwrite-delete-stale ([#2220](#2220)) ([efdd136](efdd136)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- ## [1.0.0](plugins-source-gcp-v0.9.9...plugins-source-gcp-v1.0.0) (2022-10-04) ### ⚠ BREAKING CHANGES * Official v1 release (#2335) * CloudQuery V2. (#1463) ### Features * CloudQuery V2. ([#1463](#1463)) ([d1799f3](d1799f3)) * **gcp:** Remove Classify and IgnoreError ([#1757](#1757)) ([3d34ca5](3d34ca5)) * Generate auto-filled config ([#1764](#1764)) ([2255404](2255404)) * **postgresql:** Add support for overwrite-delete-stale ([#2220](#2220)) ([efdd136](efdd136)) ### Bug Fixes * **deps:** Update golang.org/x/sync digest to 7fc1605 ([#1652](#1652)) ([daafae1](daafae1)) * **deps:** Update google.golang.org/genproto digest to c98284e ([#2171](#2171)) ([e2b5c23](e2b5c23)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.1.2 ([#1750](#1750)) ([fbe1b78](fbe1b78)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.10.2 ([#2048](#2048)) ([e407991](e407991)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.11.0 ([#2135](#2135)) ([1729467](1729467)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.11.2 ([#2162](#2162)) ([5701aa5](5701aa5)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.11.4 ([#2213](#2213)) ([1ffc1dd](1ffc1dd)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.11.5 ([#2230](#2230)) ([bd79416](bd79416)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.2.4 ([#1761](#1761)) ([7a83a65](7a83a65)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.2.5 ([#1769](#1769)) ([c9c8c05](c9c8c05)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.2.6 ([#1770](#1770)) ([5bc205e](5bc205e)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.2.7 ([#1783](#1783)) ([c291499](c291499)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.2.8 ([#1784](#1784)) ([b64e2d1](b64e2d1)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.2.9 ([#1785](#1785)) ([c6e8cb0](c6e8cb0)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.4.0 ([#1786](#1786)) ([cba274b](cba274b)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.4.1 ([#1787](#1787)) ([bad385c](bad385c)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.4.2 ([#1789](#1789)) ([79a46a2](79a46a2)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.5.0 ([#1792](#1792)) ([0b4834e](0b4834e)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.5.2 ([#1793](#1793)) ([36fd6a1](36fd6a1)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.6.0 ([#1817](#1817)) ([bd68a9c](bd68a9c)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.6.1 ([#1820](#1820)) ([2613e23](2613e23)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.6.2 ([#1838](#1838)) ([5b16c59](5b16c59)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.6.3 ([#1858](#1858)) ([9e3ace7](9e3ace7)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.6.4 ([#1862](#1862)) ([5d141cf](5d141cf)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.7.1 ([#1865](#1865)) ([474bb70](474bb70)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.7.12 ([#1916](#1916)) ([27d8153](27d8153)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.7.2 ([#1872](#1872)) ([49ed26d](49ed26d)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.7.3 ([#1886](#1886)) ([7435d59](7435d59)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.7.4 ([#1889](#1889)) ([63a5362](63a5362)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.8.0 ([#1997](#1997)) ([4fa40da](4fa40da)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.8.1 ([#2024](#2024)) ([8f88de4](8f88de4)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.8.2 ([#2044](#2044)) ([9b69b46](9b69b46)) * **deps:** Update plugin-sdk for gcp to v0.11.6 ([#2256](#2256)) ([f148b3f](f148b3f)) * **deps:** Update plugin-sdk for gcp to v0.12.2 ([#2282](#2282)) ([e0b7ba4](e0b7ba4)) * Enable GCP to find all Active Projects ([#1782](#1782)) ([5348ad4](5348ad4)) * GCP policies ([#1879](#1879)) ([1591f9f](1591f9f)) * **gcp:** Re-add gcp_iam_service_account_keys ([#2134](#2134)) ([8f419c3](8f419c3)), closes [#1990](#1990) * **gcp:** Re-add missing field in crypto_keys (rotation_period) ([#2132](#2132)) ([558ed75](558ed75)) * Naming fixes in gcp plugin ([#1903](#1903)) ([eb6ec00](eb6ec00)) * Re-add GCP project policies ([#2108](#2108)) ([1253568](1253568)), closes [#1989](#1989) * Remove unused codegen fields in GCP ([#2109](#2109)) ([f9acf5d](f9acf5d)) * Run tinypng on GCP dashboard screenshots ([#2274](#2274)) ([a2b662c](a2b662c)) * Update all source plugin to v0.12.2 ([#2316](#2316)) ([5099dcf](5099dcf)) * Used DefaultTransformer from plugin-sdk for name transformations ([#1943](#1943)) ([716ff8e](716ff8e)) ### Miscellaneous Chores * Official v1 release ([#2335](#2335)) ([e32de23](e32de23)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: erezrokah <erezrokah@users.noreply.github.com>

🤖 I have created a release *beep* *boop* --- ## [1.0.0](plugins-destination-postgresql-v0.3.0...plugins-destination-postgresql-v1.0.0) (2022-10-04) ### Features * **pg:** Add sentry ([#2139](#2139)) ([8891808](8891808)) * **postgresql:** Add support for overwrite-delete-stale ([#2220](#2220)) ([efdd136](efdd136)) ### Bug Fixes * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.11.0 ([#2135](#2135)) ([1729467](1729467)) * **deps:** Update module github.com/cloudquery/plugin-sdk to v0.11.2 ([#2162](#2162)) ([5701aa5](5701aa5)) * **deps:** Update plugin-sdk for postgresql to v0.12.2 ([#2311](#2311)) ([e4778ea](e4778ea)) * **postgresql:** Upsert in postgresql ([#2133](#2133)) ([565728b](565728b)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: Yevgeny Pats <yev.pats@gmail.com>

yevgenypats requested review from erezrokah and hermanschaaf October 2, 2022 22:06

yevgenypats requested a review from disq as a code owner October 2, 2022 22:06

yevgenypats requested a review from a team October 2, 2022 22:06

cq-bot added cli labels Oct 2, 2022

yevgenypats mentioned this pull request Oct 2, 2022

feat!: Add overwrite-delete-stale mode for destination plugins cloudquery/plugin-sdk#224

Merged

yevgenypats force-pushed the feat/overwrite_deletestale branch from edf88ee to 26fdb7f Compare October 2, 2022 22:16

yevgenypats removed request for a team and disq October 2, 2022 22:17

yevgenypats mentioned this pull request Oct 2, 2022

Deletion of Stale Data #2168

Closed

hermanschaaf suggested changes Oct 3, 2022

View reviewed changes

erezrokah requested changes Oct 3, 2022

View reviewed changes

hermanschaaf changed the title ~~feat(postgresql): Add support for overwrite-deletestale~~ feat(postgresql): Add support for overwrite-delete-stale Oct 3, 2022

yevgenypats and others added 6 commits October 3, 2022 18:43

feat(postgresql): Add support for overwrite-deletestale

c240f00

Update plugins/destination/postgresql/client/drop_test.go

be4d319

Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>

Update plugins/destination/postgresql/client/migrate_test.go

ffd6c2b

Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>

update sdk to v0.12.0

7386f28

remove old gomod replace

1335b8a

review fixes

a378fab

yevgenypats force-pushed the feat/overwrite_deletestale branch from bcfb80b to a378fab Compare October 3, 2022 15:45

yevgenypats requested review from erezrokah and hermanschaaf October 3, 2022 15:45

erezrokah approved these changes Oct 3, 2022

View reviewed changes

fix gcp linter

4735200

fix more tests

66bd359

yevgenypats added 5 commits October 3, 2022 19:15

fix gomod and gosum

3292795

fix lint

78bd9fe

fix lint

0b4637c

fix more lints

42f83d8

regen test docs

dcf0f59

Merge branch 'main' into feat/overwrite_deletestale

7d2b1e4

vercel bot deployed to Preview – cloudquery-web October 3, 2022 17:57 View deployment

yevgenypats merged commit efdd136 into main Oct 3, 2022

yevgenypats deleted the feat/overwrite_deletestale branch October 3, 2022 17:58

cq-bot mentioned this pull request Oct 4, 2022

chore(main): Release plugins-source-gcp v1.0.0 #2309

Merged

cq-bot mentioned this pull request Oct 12, 2022

chore(main): Release plugins-source-gcp v2.0.0 #2734

Closed

		replace github.com/cloudquery/plugin-sdk => ../../plugin-sdk

Conversation

yevgenypats commented Oct 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

hermanschaaf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

erezrokah left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yevgenypats Oct 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yevgenypats commented Oct 3, 2022

Uh oh!

erezrokah left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yevgenypats commented Oct 3, 2022

Uh oh!

yevgenypats commented Oct 3, 2022

Uh oh!

yevgenypats commented Oct 3, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yevgenypats commented Oct 2, 2022 •

edited

Loading

erezrokah left a comment •

edited

Loading

yevgenypats Oct 3, 2022 •

edited

Loading

erezrokah left a comment •

edited

Loading