Skip to content
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

feat(destination): bqstream - add support for batch of properties #2367

Merged
merged 14 commits into from
Oct 14, 2022

Conversation

akashrpo
Copy link
Contributor

Description

add support to accept array of properties

Notion Ticket

https://www.notion.so/rudderstacks/bqstream-Add-batching-support-8f9470b6eaac4fc1a2d750bc6827ded0

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

@akashrpo akashrpo requested a review from sanpj2292 August 29, 2022 17:50
@akashrpo akashrpo self-assigned this Aug 29, 2022
@sanpj2292
Copy link
Contributor

@akashrpo
Basically, we will need to do two things

  • Move BQStream to routerTransform
  • There will always be batching by default
  • If not, if there is a single event -> this change will break I think(cause error response). So we will need to tread this way carefully

cc: @utsabc @koladilip

@sanpj2292 sanpj2292 requested review from utsabc and koladilip August 30, 2022 00:37
@github-actions
Copy link

This PR is considered to be stale. It has been open 20 days with no further activity thus it is going to be closed in 5 days. To avoid such a case please consider removing the stale label manually or add a comment to the PR.

@github-actions github-actions bot added the Stale label Sep 20, 2022
@github-actions github-actions bot closed this Sep 27, 2022
@akashrpo akashrpo reopened this Sep 27, 2022
@akashrpo akashrpo removed the Stale label Sep 27, 2022
@akashrpo akashrpo changed the title feature(BQStream): add support for batch of properties feat(integration): bqstream - add support for batch of properties Oct 11, 2022
@akashrpo akashrpo changed the title feat(integration): bqstream - add support for batch of properties feat(destination): bqstream - add support for batch of properties Oct 11, 2022
@codecov
Copy link

codecov bot commented Oct 11, 2022

Codecov Report

Base: 43.84% // Head: 43.95% // Increases project coverage by +0.11% 🎉

Coverage data is based on head (ad08a20) compared to base (4de6105).
Patch coverage: 69.69% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2367      +/-   ##
==========================================
+ Coverage   43.84%   43.95%   +0.11%     
==========================================
  Files         187      187              
  Lines       39084    39096      +12     
==========================================
+ Hits        17136    17186      +50     
+ Misses      20864    20825      -39     
- Partials     1084     1085       +1     
Impacted Files Coverage Δ
services/streammanager/bqstream/bqstreammanager.go 71.59% <69.69%> (+38.69%) ⬆️
config/backend-config/namespace_config.go 70.83% <0.00%> (-3.13%) ⬇️
warehouse/schema.go 48.98% <0.00%> (+1.15%) ⬆️
enterprise/reporting/reporting.go 9.77% <0.00%> (+1.43%) ⬆️
enterprise/suppress-user/suppressUser.go 82.71% <0.00%> (+1.85%) ⬆️
enterprise/reporting/setup.go 52.38% <0.00%> (+14.28%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

@koladilip koladilip force-pushed the feature/bqstream-batch-support branch from 6926586 to 93eab29 Compare October 12, 2022 16:07
@koladilip
Copy link
Contributor

@akashrpo Please add tests for Produce then we can merge.

@akashrpo akashrpo force-pushed the feature/bqstream-batch-support branch from d7dee7f to ad08a20 Compare October 14, 2022 04:31
@koladilip
Copy link
Contributor

@akashrpo if all checks are passed you can merge

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