feat: Add Transformer for tables (codegen replacement)#564
feat: Add Transformer for tables (codegen replacement)#564kodiakhq[bot] merged 5 commits intomainfrom
Conversation
⏱️ Benchmark results
|
hermanschaaf
left a comment
There was a problem hiding this comment.
Looks good, would definitely be nice to not need codegen! I guess developers will still have to rely on docs generation to see what the output is though, so there is still some codegen in the loop.
I'm also curious to see what the performance of this will be like with hundreds of tables, and whether it will be good enough to run without some kind of caching (cough codegen cough) step :)
Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
There was a problem hiding this comment.
Looks good 🚀
Same concerns as @hermanschaaf as we're moving the generation to runtime and also lose some visibility on what's generated.
We should probably have some test that runs on all tables and ensures the transformation works (we don't have mock tests coverage for all plugins). That test could also serve as a benchmark to know how much time it takes to start a plugin
Also to confirm - this removes only the table codegen, but we still need resolvers codegen in each plugin correct?
| @@ -0,0 +1,151 @@ | |||
| package transformers | |||
|
Apart from the missing modifiers to add |
Co-authored-by: Kemal <223029+disq@users.noreply.github.com>
Indeed this is the downside but to understand what the transformation does it generate the markdown which shows exactly what columns are created after transformation. The otherway around creates an unmaintainable codegen code. |
🤖 I have created a release *beep* *boop* --- ## [1.18.0](v1.17.2...v1.18.0) (2023-01-04) ### Features * Add Transformer for tables (codegen replacement) ([#564](#564)) ([a643ddf](a643ddf)) * Support conversion of Unix timestamps in timestamptz ([#570](#570)) ([6b948ab](6b948ab)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This shouldn't introduce any public facing changes. This should go together with: cloudquery/plugin-sdk#564 In general codegen creates a sub-optimal developer experience where you have to juggle between `codegen` and code to make any code-changes. Not only it creates a tough experience for the developer it also makes it hard to review and to contribute. This take here moves codegen only to the discovery phase and moves the transformation to be part of the SDK which will eventually deprecate the `codegen` package from the SDK completely and will avoid having two APIs which are almost identical but quite the same.
Similar to #6276 moving transformation to be part of our SDK cloudquery/plugin-sdk#564 This is related to cloudquery/plugin-sdk#573 Few notes on column changes: most of changes are just additional columns and this should be fine as we did too much transformation. we want to keep the code simple, focus on extraction and provide and easy way of doing "dumb" one level transformation via reflection. The additional column are because we have in some places a duplicate where we add our own `arn` so it can be consistent but this is our decision and we still want to be as close as possible to the API so some duplicates is fine (and it makes the code simple and consistent). breaking changes in `aws_docdb_pending_maintenance_actions ` those apparently never worked and those columns where always empty (they didn't have mock tests so we didn't catch those) Co-authored-by: Kemal <223029+disq@users.noreply.github.com> Co-authored-by: Kemal Hadimli <disq@users.noreply.github.com>
No description provided.