Skip to content

refactor: Use status.Convert instead of FromError#484

Merged
kodiakhq[bot] merged 2 commits intocloudquery:mainfrom
erezrokah:refactor/simplify_from_error
Dec 24, 2022
Merged

refactor: Use status.Convert instead of FromError#484
kodiakhq[bot] merged 2 commits intocloudquery:mainfrom
erezrokah:refactor/simplify_from_error

Conversation

@erezrokah
Copy link
Copy Markdown
Member

Summary

Similar to #478.
status.Convert calls FromError internally.
Some notes:

  1. No need to check s != nil as status.Convert always returns a non nil value if err != nil
  2. FromError returns false if err is not a gRPC error. This can never happen in this case. Also I believe the failed to call GetProtocolVersion is wrong as it should be err is not a gRPC error. Regardless I don't think we need it

Use the following steps to ensure your PR is ready to be reviewed

  • Read the contribution guidelines 🧑‍🎓
  • Run go fmt to format your code 🖊
  • Lint your changes via golangci-lint run 🚨 (install golangci-lint here)
  • Update or add tests 🧪
  • Ensure the status checks below are successful ✅

@github-actions
Copy link
Copy Markdown

⏱️ Benchmark results

Comparing with ad20787

  • DefaultConcurrency-2 resources/s: 12,109 ⬆️ 6.83% increase vs. ad20787
  • Glob-2 ns/op: 159 ⬆️ 5.03% increase vs. ad20787
  • TablesWithChildrenDefaultConcurrency-2 resources/s: 29,870 ⬇️ 3.28% decrease vs. ad20787
  • BufferedScanner-2 ns/op: 9.382 ⬆️ 1.11% increase vs. ad20787
  • LogReader-2 ns/op: 30.72 ⬆️ 0.42% increase vs. ad20787

@kodiakhq kodiakhq bot merged commit 30d07ff into cloudquery:main Dec 24, 2022
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