Skip to content

fix: Don't send migrate messages in destination v1 write#1167

Merged
kodiakhq[bot] merged 2 commits intocloudquery:mainfrom
erezrokah:fix/dont_run_migrate_inside_v1_destination_write
Aug 28, 2023
Merged

fix: Don't send migrate messages in destination v1 write#1167
kodiakhq[bot] merged 2 commits intocloudquery:mainfrom
erezrokah:fix/dont_run_migrate_inside_v1_destination_write

Conversation

@erezrokah
Copy link
Copy Markdown
Member

@erezrokah erezrokah commented Aug 25, 2023

Summary

Migrate messages are already sent in the CLI here https://github.com/cloudquery/cloudquery/blob/eab200d6944c54db5ef6d9a81b290ed6b8ddd06f/cli/cmd/sync_v2.go#L145 (during sync) and here https://github.com/cloudquery/cloudquery/blob/eab200d6944c54db5ef6d9a81b290ed6b8ddd06f/cli/cmd/migrate_v2.go#L66 (during migrate).

Without this fix, if you have a v2 source and a v3 destination sync will run migration twice, and sync --no-migrate will run migration once


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 ✅

@codecov
Copy link
Copy Markdown

codecov bot commented Aug 25, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.04% ⚠️

Comparison is base (badec9d) 48.75% compared to head (688f85f) 48.71%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1167      +/-   ##
==========================================
- Coverage   48.75%   48.71%   -0.04%     
==========================================
  Files          87       87              
  Lines        8086     8080       -6     
==========================================
- Hits         3942     3936       -6     
  Misses       3787     3787              
  Partials      357      357              
Files Changed Coverage Δ
internal/servers/destination/v1/destinations.go 22.85% <ø> (-2.56%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions github-actions bot added fix and removed fix labels Aug 25, 2023
@github-actions
Copy link
Copy Markdown

⏱️ Benchmark results

Comparing with badec9d

  • Glob-8 ns/op: 99.78 ⬆️ 0.46% increase vs. badec9d

@kodiakhq kodiakhq bot merged commit 9ed543c into cloudquery:main Aug 28, 2023
@erezrokah erezrokah deleted the fix/dont_run_migrate_inside_v1_destination_write branch August 28, 2023 08:22
kodiakhq bot pushed a commit that referenced this pull request Aug 28, 2023
🤖 I have created a release *beep* *boop*
---


## [4.5.6](v4.5.5...v4.5.6) (2023-08-28)


### Bug Fixes

* Don't send migrate messages in destination v1 write ([#1167](#1167)) ([9ed543c](9ed543c))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
@github-actions github-actions bot added fix and removed fix labels Aug 28, 2023
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.

2 participants