rudder-sdk-js icon indicating copy to clipboard operation
rudder-sdk-js copied to clipboard

add events batch dispatching mode

Open JavadHosseini opened this issue 2 years ago • 5 comments

Description of the change

The batch mode allows you to send a series of identify, track, page, group and screen requests in a single batch. This call helps you minimize the number of outbound requests, thus enabling better performance. Two load option has been added:

  • batchMode: boolean
  • batchFactor: Integer

Type of change

  • [ ] Bug fix (non-breaking change that fixes an issue)
  • [x] New feature (non-breaking change that adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

Related issues

N/A

Checklists

Development

  • [ ] Lint rules pass locally
  • [ ] The code changed/added as part of this pull request has been covered with tests
  • [ ] All tests related to the changed code pass in development

Code review

  • [ ] This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
  • [ ] "Ready for review" label attached to the PR and reviewers mentioned in a comment
  • [ ] Changes have been reviewed by at least one other engineer
  • [ ] Issue from task tracker has a link to this pull request

This change is Reviewable

JavadHosseini avatar Jun 02 '22 14:06 JavadHosseini

@JavadHosseini Thanks for raising the PR. We've added this to our SDK roadmap. However, as we are swamped currently, please expect things to go at a slower pace.

saikumarrs avatar Jun 09 '22 11:06 saikumarrs

As a next step, please sign the CLA using this form.

saikumarrs avatar Jun 09 '22 11:06 saikumarrs

@saikumarrs If I publish this change as a npm package until you add this feature, is it ok? we need this change in our company.

AliTaee avatar Jun 26 '22 10:06 AliTaee

@saikumarrs If I publish this change as a npm package until you add this feature, is it ok? we need this change in our company.

@AliTaee Ideally, we would want the CDN and NPM versions of the SDK to be alike. We'll add this to our current sprint and try to release it in the next 2 weeks. While we work on this, please resolve the conflicts in your branch. If you haven't already, please sign the CLA using this form.

saikumarrs avatar Jun 26 '22 12:06 saikumarrs

If you are not concerned about the event retries on failures, I would recommend using the beacon transportation mode.

saikumarrs avatar Jun 26 '22 13:06 saikumarrs

@saikumarrs I resolved the conflicts, I hope it would be OK. Can you place it in your next release? We are about to deploy this changes on our servers. Thanks.

JavadHosseini avatar Sep 25 '22 10:09 JavadHosseini

@saikumarrs I resolved the conflicts, I hope it would be OK. Can you place it in your next release? We are about to deploy this changes on our servers. Thanks.

@JavadHosseini Thanks for making changes. We discussed this internally and decided to support this feature more robustly. Moreover, we also wanted to add this for the Beacon transport mechanism as well. As it would big change, it needs to be thought out well instead of isolated changes like these. We'll add this to our roadmap, determining the exact scope this quarter and implementing it in early Q1. We regret any inconvenience caused.

saikumarrs avatar Sep 26 '22 12:09 saikumarrs

@saikumarrs I resolved the conflicts, I hope it would be OK. Can you place it in your next release? We are about to deploy this changes on our servers. Thanks.

@JavadHosseini Thanks for making changes. We discussed this internally and decided to support this feature more robustly. Moreover, we also wanted to add this for the Beacon transport mechanism as well. As it would big change, it needs to be thought out well instead of isolated changes like these. We'll add this to our roadmap, determining the exact scope this quarter and implementing it in early Q1. We regret any inconvenience caused.

Great, thanks, So I guess we deploy my changes and then after the official release we go for that...

JavadHosseini avatar Sep 29 '22 12:09 JavadHosseini

@saikumarrs I resolved the conflicts, I hope it would be OK. Can you place it in your next release? We are about to deploy this changes on our servers. Thanks.

@JavadHosseini Thanks for making changes. We discussed this internally and decided to support this feature more robustly. Moreover, we also wanted to add this for the Beacon transport mechanism as well. As it would big change, it needs to be thought out well instead of isolated changes like these. We'll add this to our roadmap, determining the exact scope this quarter and implementing it in early Q1. We regret any inconvenience caused.

Great, thanks, So I guess we deploy my changes and then after the official release we go for that...

@JavadHosseini Thanks for your cooperation. I'll go ahead and close the PR for now.

saikumarrs avatar Sep 29 '22 13:09 saikumarrs