Skip to content

feat: Add unwrap option to transformations#573

Merged
kodiakhq[bot] merged 1 commit intomainfrom
fix/transform_unwrap
Jan 5, 2023
Merged

feat: Add unwrap option to transformations#573
kodiakhq[bot] merged 1 commit intomainfrom
fix/transform_unwrap

Conversation

@yevgenypats
Copy link
Copy Markdown
Contributor

@yevgenypats yevgenypats commented Jan 4, 2023

Follow-up on transformation. Adding unwrap option as needed in some cases where we create our own wrapper struct.

This also moves the tests from codegen.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 4, 2023

⏱️ Benchmark results

Comparing with d89a911

  • DefaultConcurrencyDFS-2 resources/s: 10,857 ⬆️ 1.08% increase vs. d89a911
  • DefaultConcurrencyRoundRobin-2 resources/s: 10,844 ⬇️ 15.07% decrease vs. d89a911
  • Glob-2 ns/op: 156.9 ⬆️ 4.02% increase vs. d89a911
  • TablesWithChildrenDFS-2 resources/s: 28,834 ⬇️ 7.49% decrease vs. d89a911
  • TablesWithChildrenRoundRobin-2 resources/s: 27,740 ⬇️ 4.32% decrease vs. d89a911
  • TablesWithRateLimitingDFS-2 resources/s: 28.24 ⬇️ 1.03% decrease vs. d89a911
  • TablesWithRateLimitingRoundRobin-2 resources/s: 798.5 ⬇️ 5.81% decrease vs. d89a911
  • BufferedScanner-2 ns/op: 13.46 ⬆️ 25.26% increase vs. d89a911
  • LogReader-2 ns/op: 30.78 ⬆️ 0.94% increase vs. d89a911

Copy link
Copy Markdown
Member

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, maybe we can do another PR to add the WithPrimaryKeys option?

@yevgenypats
Copy link
Copy Markdown
Contributor Author

Looks good, maybe we can do another PR to add the WithPrimaryKeys option?

I want to avoid it for now. The way to override in the paradaigm is just to enter it manually in the column otherwise we will end up WithPKNewType. WithOverrideType and so on...

@kodiakhq kodiakhq bot merged commit a17ee4b into main Jan 5, 2023
@kodiakhq kodiakhq bot deleted the fix/transform_unwrap branch January 5, 2023 08:12
kodiakhq bot pushed a commit that referenced this pull request Jan 5, 2023
🤖 I have created a release *beep* *boop*
---


## [1.19.0](v1.18.0...v1.19.0) (2023-01-05)


### Features

* Add scheduler option and introduce Round Robin scheduler ([#545](#545)) ([d89a911](d89a911))
* Add unwrap option to transformations ([#573](#573)) ([a17ee4b](a17ee4b))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
yevgenypats added a commit to cloudquery/cloudquery that referenced this pull request Jan 9, 2023
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants