Skip to content

fix(transformers): Ability to transform any with TypeTransformer#956

Merged
kodiakhq[bot] merged 4 commits intocloudquery:mainfrom
disq:fix/ability_to_transform_any
Jun 12, 2023
Merged

fix(transformers): Ability to transform any with TypeTransformer#956
kodiakhq[bot] merged 4 commits intocloudquery:mainfrom
disq:fix/ability_to_transform_any

Conversation

@disq
Copy link
Member

@disq disq commented Jun 7, 2023

plus testing fixes.

Without this there was no way to transform any (unless it's defined as []any) because bare interfaces were ignored in isTypeIgnored() along with channels/funcs/unsafeptrs.

@disq disq requested a review from hermanschaaf June 7, 2023 11:00
@disq disq requested a review from yevgenypats as a code owner June 7, 2023 11:00
@github-actions github-actions bot added the fix label Jun 7, 2023
@github-actions
Copy link

github-actions bot commented Jun 7, 2023

⏱️ Benchmark results

  • DefaultConcurrencyDFS-2 resources/s: 10,693
  • DefaultConcurrencyRoundRobin-2 resources/s: 11,077
  • Glob-2 ns/op: 224.9
  • TablesWithChildrenDFS-2 resources/s: 26,160
  • TablesWithChildrenRoundRobin-2 resources/s: 26,789
  • TablesWithRateLimitingDFS-2 resources/s: 28.52
  • TablesWithRateLimitingRoundRobin-2 resources/s: 819.7

@codecov
Copy link

codecov bot commented Jun 7, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.13 🎉

Comparison is base (8c7acdd) 52.68% compared to head (ae9a567) 52.81%.

❗ Current head ae9a567 differs from pull request most recent head 5d44fe4. Consider uploading reports for the commit 5d44fe4 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #956      +/-   ##
==========================================
+ Coverage   52.68%   52.81%   +0.13%     
==========================================
  Files          62       62              
  Lines        6332     6337       +5     
==========================================
+ Hits         3336     3347      +11     
+ Misses       2691     2685       -6     
  Partials      305      305              
Impacted Files Coverage Δ
transformers/struct.go 76.03% <100.00%> (+3.03%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

}
}
if columnType == nil {
return nil // ignored
Copy link
Member Author

Choose a reason for hiding this comment

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

This ensures backwards compatibility with the current situation where we ignore reflect.Interface early in isTypeIgnored.

return arrow.ListOf(elemValueType), nil

case reflect.Interface:
return nil, nil // silently ignore
Copy link
Member Author

Choose a reason for hiding this comment

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

This ensures we don't panic now that we don't ignore this kind in isTypeIgnored.

}
for _, exc := range tt.want.Columns {
if c := table.Column(exc.Name); c == nil {
t.Fatalf("column %q not found. want: %v", exc.Name, exc.Type)
Copy link
Member

Choose a reason for hiding this comment

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

Nice 👍

@disq
Copy link
Member Author

disq commented Jun 7, 2023

Ran this on all source plugins - no changes.

@disq disq added the automerge label Jun 7, 2023
@kodiakhq kodiakhq bot merged commit c989c28 into cloudquery:main Jun 12, 2023
@disq disq deleted the fix/ability_to_transform_any branch June 12, 2023 09:38
kodiakhq bot pushed a commit that referenced this pull request Jun 13, 2023
🤖 I have created a release *beep* *boop*
---


## [3.10.5](v3.10.4...v3.10.5) (2023-06-13)


### Bug Fixes

* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to 0f7bd3b ([#961](#961)) ([21f3b68](21f3b68))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to 6b7fa9c ([#962](#962)) ([78eecf2](78eecf2))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to 71dfe94 ([#953](#953)) ([b48ae1a](b48ae1a))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to 7f6aaff ([#963](#963)) ([8c7acdd](8c7acdd))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to 8f72077 ([#958](#958)) ([6f6c993](6f6c993))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to 90670b8 ([#955](#955)) ([047ab30](047ab30))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to b359e74 ([#960](#960)) ([7e95e7d](7e95e7d))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to d8eacf8 ([#966](#966)) ([2d32679](2d32679))
* **deps:** Update github.com/cloudquery/arrow/go/v13 digest to e258cfb ([#957](#957)) ([df842e0](df842e0))
* **transformers:** Ability to transform `any` with TypeTransformer ([#956](#956)) ([c989c28](c989c28))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
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.

3 participants