Skip to content

feat: Time values are truncated uniformly #825

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 27, 2023

Conversation

bbernays
Copy link
Contributor

Summary

Make sure time values are truncated uniformly across all operating systems and in line with the arrow casting

@bbernays bbernays requested a review from yevgenypats as a code owner April 25, 2023 23:40
@github-actions github-actions bot added the feat label Apr 25, 2023
@github-actions
Copy link

github-actions bot commented Apr 25, 2023

⏱️ Benchmark results

Comparing with 4cd3872

  • DefaultConcurrencyDFS-2 resources/s: 10,220 ⬆️ 2.80% increase vs. 4cd3872
  • DefaultConcurrencyRoundRobin-2 resources/s: 11,803 ⬇️ 4.42% decrease vs. 4cd3872
  • Glob-2 ns/op: 155.8 ⬇️ 23.68% decrease vs. 4cd3872
  • TablesWithChildrenDFS-2 resources/s: 27,297 ⬇️ 5.03% decrease vs. 4cd3872
  • TablesWithChildrenRoundRobin-2 resources/s: 29,204 ⬆️ 11.51% increase vs. 4cd3872
  • TablesWithRateLimitingDFS-2 resources/s: 28.27 ⬆️ 0.42% increase vs. 4cd3872
  • TablesWithRateLimitingRoundRobin-2 resources/s: 839.3 ⬇️ 2.23% decrease vs. 4cd3872
  • BufferedScanner-2 ns/op: 9.381 ⬆️ 0.05% increase vs. 4cd3872
  • LogReader-2 ns/op: 30.68 ⬆️ 0.03% increase vs. 4cd3872

@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

Patch coverage: 33.33% and project coverage change: -0.22 ⚠️

Comparison is base (4cd3872) 47.19% compared to head (8cc61d3) 46.98%.

❗ Current head 8cc61d3 differs from pull request most recent head b307f91. Consider uploading reports for the commit b307f91 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #825      +/-   ##
==========================================
- Coverage   47.19%   46.98%   -0.22%     
==========================================
  Files          76       76              
  Lines        7844     7873      +29     
==========================================
- Hits         3702     3699       -3     
- Misses       3642     3683      +41     
+ Partials      500      491       -9     
Impacted Files Coverage Δ
clients/destination/v0/destination.go 36.12% <33.33%> (-0.14%) ⬇️

... and 4 files with indirect coverage changes

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

@disq
Copy link
Member

disq commented Apr 26, 2023

Could also be done in the CLI: cloudquery/cloudquery#10365

@bbernays
Copy link
Contributor Author

Could also be done in the CLI: cloudquery/cloudquery#10365

Agreed, but the issue still exists for anyone that doesn't upgrade their CLI... This way as long as they can update their destination and be safe

@bbernays bbernays requested review from hermanschaaf and disq April 26, 2023 13:00
@bbernays bbernays requested a review from disq April 26, 2023 17:21
@disq disq merged commit ffb97b0 into cloudquery:main Apr 27, 2023
candiduslynx pushed a commit that referenced this pull request Apr 28, 2023
🤖 I have created a release *beep* *boop*
---


##
[2.5.0](v2.4.0...v2.5.0)
(2023-04-28)


### Features

* Add table description to Arrow schema metadata
([#824](#824))
([1a8072f](1a8072f))
* **arrow:** Streamline Apache Arrow extension types
([#823](#823))
([f32fac3](f32fac3))
* **test:** Add double migration test
([#827](#827))
([4cd3872](4cd3872))
* Time values are truncated uniformly
([#825](#825))
([ffb97b0](ffb97b0))


### Bug Fixes

* TransformWithStruct/DefaultNameTransformer change for invalid column
names ([#820](#820))
([01e6649](01e6649))

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