Skip to content

fix: Don't panic on empty-string for timestamp#489

Merged
kodiakhq[bot] merged 2 commits intocloudquery:mainfrom
shimonp21:fix_timestamp_from_empty_string
Dec 12, 2022
Merged

fix: Don't panic on empty-string for timestamp#489
kodiakhq[bot] merged 2 commits intocloudquery:mainfrom
shimonp21:fix_timestamp_from_empty_string

Conversation

@shimonp21
Copy link
Copy Markdown
Contributor

@shimonp21 shimonp21 commented Dec 12, 2022

Summary

Fixes this panic:

12:22PM ERR column resolver finished with panic error={} client=pagerduty column=created_at duration=0.138344 module=pagerduty-src stack="failed to set column created_at: cannot parse  as Timestamptz\ngoroutine 402 [running]:\nruntime/debug.Stack()\n\t/usr/local/Cellar/go/1.19.1/libexec/src/runtime/debug/stack.go:24 +0x65\ngithub.com/cloudquery/plugin-sdk/plugins.(*SourcePlugin).resolveColumn.func1()\n\t/Users/shimonp/projects/plugin-sdk-shimonp21/plugins/source_scheduler_dfs.go:226 +0x86\npanic({0x163c740, 0xc00049a860})\n\t/usr/local/Cellar/go/1.19.1/libexec/src/runtime/panic.go:884 +0x212\ngithub.com/cloudquery/plugin-sdk/schema.(*Resource).Set(0xc0005e8740?, {0x16f71e7, 0xa}, {0x15fb260?, 0xc0000e6158?})\n\t/Users/shimonp/projects/plugin-sdk-shimonp21/schema/resource.go:74 +0x155\ngithub.com/cloudquery/plugin-sdk/schema.PathResolver.func1({0x18?, 0x1676020?}, {0xc0000be001?, 0xc0003a1a58?}, 0xc0005e8740, {{0x16f71e7, 0xa}, 0x9, {0x0, 0x0}, ...})\n\t/Users/shimonp/projects/plugin-sdk-shimonp21/schema/resolvers.go:17 +0x73\ngithub.com/cloudquery/plugin-sdk/plugins.(*SourcePlugin).resolveColumn(0xc000294000, {0x17f5398, 0xc000516e40}, {{0x17f3878, 0xc00026a470}, 0x1, {0x0, 0x0}, {0xc000418a00, 0x5a, ...}, ...}, ...)\n\t/Users/shimonp/projects/plugin-sdk-shimonp21/plugins/source_scheduler_dfs.go:233 +0x1cd\ngithub.com/cloudquery/plugin-sdk/plugins.(*SourcePlugin).resolveResource(0xc000294000, {0x17f53d0?, 0xc000319b60?}, 0xc0001f8f80, {0x17ef700, 0xc00034a7e0}, 0x14?, {0x16c49c0, 0xc0000e6100})\n\t/Users/shimonp/projects/plugin-sdk-shimonp21/plugins/source_scheduler_dfs.go:209 +0x805\ngithub.com/cloudquery/plugin-sdk/plugins.(*SourcePlugin).resolveResourcesDfs.func1.1()\n\t/Users/shimonp/projects/plugin-sdk-shimonp21/plugins/source_scheduler_dfs.go:148 +0x105\ncreated by github.com/cloudquery/plugin-sdk/plugins.(*SourcePlugin).resolveResourcesDfs.func1\n\t/Users/shimonp/projects/plugin-sdk-shimonp21/plugins/source_scheduler_dfs.go:144 +0xf6\n" table=pagerduty_user_notification_rules

@github-actions github-actions bot added the fix label Dec 12, 2022
@disq
Copy link
Copy Markdown
Member

disq commented Dec 12, 2022 •
edited
Loading

Slices should never be checked for nil but len anyway. Could've been an empty byte slice ([]byte{}) and would also fail.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 12, 2022

⏱️ Benchmark results

Comparing with 1e2b708

  • DefaultConcurrency-2 resources/s: 10,241 ⬇️ 17.41% decrease vs. 1e2b708
  • Glob-2 ns/op: 195.9 ⬇️ 7.10% decrease vs. 1e2b708
  • TablesWithChildrenDefaultConcurrency-2 resources/s: 29,183 ⬆️ 10.80% increase vs. 1e2b708
  • BufferedScanner-2 ns/op: 11.76 ⬇️ 13.01% decrease vs. 1e2b708
  • LogReader-2 ns/op: 36.77 ⬇️ 9.74% decrease vs. 1e2b708

@kodiakhq kodiakhq bot merged commit 83813de into cloudquery:main Dec 12, 2022
@cq-bot cq-bot mentioned this pull request Dec 12, 2022
Merged
erezrokah pushed a commit that referenced this pull request Dec 13, 2022
@cq-bot
chore(main): Release v1.12.1 (#490)
845ac1c 
🤖 I have created a release *beep* *boop*
---


##
[1.12.1](v1.12.0...v1.12.1)
(2022-12-13)


### Bug Fixes

* Don't panic on empty-string for timestamp
([#489](#489))
([83813de](83813de))
* Fix deadlock off-by-one
([#493](#493))
([4ea9ed8](4ea9ed8))
* Reduce default concurrency
([#491](#491))
([f995da9](f995da9))
* Refactor glob filters
([#488](#488))
([cb5f6bb](cb5f6bb))

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

Reviewers

@disq disq disq approved these changes

+1 more reviewer

@yevgenypats yevgenypats yevgenypats approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

automerge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@shimonp21 @disq @yevgenypats