Skip to content

Conversation

@disq
Copy link
Member

@disq disq commented Jan 9, 2023

GCP: metadata, binary_data and etag fields are being switched from IntArray to ByteArray
K8s: spec_request, status_certificate switched from IntArray to ByteArray
AWS: aws_amp_workspaces.data, aws_iam_virtual_mfa_devices.base32_string_seed and qr_code_png
Azure: No changes

@disq disq requested a review from yevgenypats as a code owner January 9, 2023 14:19
@github-actions github-actions bot added the fix label Jan 9, 2023
@github-actions
Copy link

github-actions bot commented Jan 9, 2023

⏱️ Benchmark results

Comparing with a54dbe7

  • DefaultConcurrencyDFS-2 resources/s: 11,120 ⬆️ 0.54% increase vs. a54dbe7
  • DefaultConcurrencyRoundRobin-2 resources/s: 12,501 ⬆️ 13.29% increase vs. a54dbe7
  • Glob-2 ns/op: 159.4 ⬆️ 2.07% increase vs. a54dbe7
  • TablesWithChildrenDFS-2 resources/s: 27,746 ⬇️ 8.67% decrease vs. a54dbe7
  • TablesWithChildrenRoundRobin-2 resources/s: 29,282 ⬆️ 4.03% increase vs. a54dbe7
  • TablesWithRateLimitingDFS-2 resources/s: 28.88 ⬆️ 1.66% increase vs. a54dbe7
  • TablesWithRateLimitingRoundRobin-2 resources/s: 832.5 ⬇️ 0.02% decrease vs. a54dbe7
  • BufferedScanner-2 ns/op: 10 ⬆️ 6.16% increase vs. a54dbe7
  • LogReader-2 ns/op: 30.78 ⬆️ 0.26% increase vs. a54dbe7

return schema.TypeJSON, nil
case reflect.Slice:
switch v.Elem().Kind() {
case reflect.Uint8:
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be in defaultGoTypeToSchemaType?

Copy link
Contributor

Choose a reason for hiding this comment

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

defaultGoTypeToSchemaType will return TypeInt in this case, effectively resulting in TypeIntArray returned

Copy link
Member Author

Choose a reason for hiding this comment

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

there's no schema.TypeByte, so no.

@github-actions github-actions bot added fix and removed fix labels Jan 9, 2023
Copy link
Contributor

@yevgenypats yevgenypats left a comment

Choose a reason for hiding this comment

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

Can we add a regression test for that?

@disq
Copy link
Member Author

disq commented Jan 9, 2023

Can we add a regression test for that?

@yevgenypats added

@disq disq added the automerge label Jan 9, 2023
@kodiakhq kodiakhq bot merged commit 73ea82c into cloudquery:main Jan 9, 2023
@disq disq deleted the fix/arraytypes branch January 9, 2023 20:06
kodiakhq bot pushed a commit that referenced this pull request Jan 10, 2023
🤖 I have created a release *beep* *boop*
---


## [1.24.1](v1.24.0...v1.24.1) (2023-01-09)


### Bug Fixes

* Array types ([#587](#587)) ([73ea82c](73ea82c))
* Sentry errors not sent ([#592](#592)) ([9f1e373](9f1e373))

---
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.

4 participants