feat: Add option to plugin doc command to output tables as JSON by hermanschaaf · Pull Request #347 · cloudquery/plugin-sdk

hermanschaaf · 2022-11-04T16:37:44Z

Fixes #309

This adds a --format json option to the source plugin doc command, which allows all tables to be dumped as a single JSON file. This can then be further parsed with jq for various purposes (such as comparing two versions of a plugin to see what changed).

Example: List first 5 top-level tables of AWS plugin

./aws doc --format json .

$ cat __tables.json | jq -r '.[].name' | head -n 5
aws_accessanalyzer_analyzers
aws_acm_certificates
aws_apigateway_api_keys
aws_apigateway_client_certificates
aws_apigateway_domain_names

plugins/source_docs.go

yevgenypats

Sorry for the delayed review, forgot about it. It looks good. One nit and one question.

yevgenypats · 2022-11-14T14:24:12Z

plugins/source_docs.go

+	return string(s)
+}
+
+func (s SourceDocsFormat) Validate() error {


I think this can be just an enum (https://github.com/cloudquery/plugin-sdk/blob/main/serve/source.go#L67) which already include the validation.

Good point, but I think the enum code is private to the serve package so I'll have to refactor a bit first.

yevgenypats · 2022-11-14T14:28:54Z

plugins/source_docs.go

+	}
+}
+
+type jsonTable struct {


I think this is fine but can't we re-use the same tables definition that we already have? we also encode them to json or we want a different structure.

I tried this initially, and if I recall correctly the issue I faced was that column type came out as an integer, which isn't very useful without the mappings. So I figured that it would be good to decouple the GRPC representation from the one we use here, but I could be convinced that we shouldn't do that, and figure out a way to turn the type to a string here instead?

Yeah I don't have a good idea right now so maybe we can merge this as is and maybe fix later on if it becomes a maintains burden. But actually now that I think about it why the gRPC representation is not good ? especially if it's intended to be read by computers? Maybe we can have some flag that will marshal the Type in a different way.

I think one use case for this is going to be outputting the schema in a different format than the markdown we support. Maybe a different markdown format, maybe HTML? But having a number for the type is not very useful then, you'd either need access to the plugin-sdk or copy-paste the list of constants to know how it maps to a string. I also think it's probably good if we can evolve the internal representations separately. We may even move away from JSON for internal communication purposes if it turns out to be a drag on performance, but we'd still want to keep this.

yevgenypats · 2022-11-15T14:15:19Z

@hermanschaaf I think we can do just the enum and we can merge it/roll this out so user can use it.

…put-tables

hermanschaaf · 2022-11-15T15:55:04Z

@yevgenypats Updated to use enums, PTAL

erezrokah · 2022-11-16T13:21:24Z

plugins/.snapshots/TestGenerateSourcePluginDocs-JSON-__tables.json

+        "is_primary_key": true
+      }
+    ],
+    "relations": [


Sorry for jumping late to this, but can we also have the relations at the top level? Might be easier to process it that way. For example if I was to get a list of all tables, I'll need to recurse into the JSON at the moment.

We can add a another property parent to link to the parent table, and keep it nil for top level ones, WDYT?

Yeah, I was in two minds about it 🤔 I think having it flattened will be good for some use cases, and not-so-good for others. I'm fine with changing it to flattened though. @yevgenypats WDYT?

Maybe we can have both?

🤖 I have created a release *beep* *boop* --- ## [1.6.0](v1.5.3...v1.6.0) (2022-11-21) ### Features * Add option to plugin doc command to output tables as JSON ([#347](#347)) ([c1b4240](c1b4240)) * Support ${file:./path} expansion in spec ([#418](#418)) ([58d7c44](58d7c44)) ### Bug Fixes * Fix Destination testing suite ([#417](#417)) ([4771efa](4771efa)) * Increase GRPC message size limit to 50MiB ([#419](#419)) ([a54c6ea](a54c6ea)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

hermanschaaf added 2 commits November 4, 2022 16:24

Add option to plugin doc command to output tables as JSON

53a50aa

Rename to __tables.json and fix tests

3d75ec9

hermanschaaf requested a review from yevgenypats as a code owner November 4, 2022 16:37

github-actions bot added the feat label Nov 4, 2022

hermanschaaf commented Nov 4, 2022

View reviewed changes

plugins/source_docs.go Show resolved Hide resolved

Remove description from columns because it's currently unused

a077f86

hermanschaaf mentioned this pull request Nov 4, 2022

chore(ci): Add workflow that adds a summary on changed/updated resources cloudquery/cloudquery#3557

Closed

Satisfy govet

e25cf75

github-actions bot added feat and removed feat labels Nov 6, 2022

disq reviewed Nov 7, 2022

View reviewed changes

plugins/source_docs.go Outdated Show resolved Hide resolved

hermanschaaf and others added 2 commits November 7, 2022 09:24

Add primary key to column output

03bc6f9

Merge branch 'main' into json-output-tables

9d4dc52

disq approved these changes Nov 8, 2022

View reviewed changes

yevgenypats suggested changes Nov 14, 2022

View reviewed changes

erezrokah mentioned this pull request Nov 15, 2022

feat(website): Allow searching for supported resources/tables cloudquery/cloudquery#4632

Closed

hermanschaaf added 3 commits November 15, 2022 15:19

Merge branch 'main' of github.com:cloudquery/plugin-sdk into json-out…

592baa0

…put-tables

use enum

0cd7b4d

Update test

a218714

Merge branch 'main' into json-output-tables

772638d

erezrokah reviewed Nov 16, 2022

View reviewed changes

yevgenypats approved these changes Nov 16, 2022

View reviewed changes

yevgenypats added the automerge label Nov 16, 2022

kodiakhq bot merged commit c1b4240 into main Nov 16, 2022

kodiakhq bot deleted the json-output-tables branch November 16, 2022 13:46

cq-bot mentioned this pull request Nov 16, 2022

chore(main): Release v1.6.0 #412

Merged

erezrokah mentioned this pull request Nov 16, 2022

feat(website): Add source plugins tables cloudquery/cloudquery#4728

Closed

yevgenypats mentioned this pull request Nov 23, 2022

output requested tables as json cloudquery/cloudquery#5003

Closed

Conversation

hermanschaaf commented Nov 4, 2022 • edited by erezrokah Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Example: List first 5 top-level tables of AWS plugin

Uh oh!

Uh oh!

Uh oh!

yevgenypats left a comment

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!

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 commented Nov 15, 2022

Uh oh!

hermanschaaf commented Nov 15, 2022

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hermanschaaf commented Nov 4, 2022 •

edited by erezrokah

Loading