feat(hs): chunking data based on batch limit #2907
Conversation
|
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the WalkthroughThe recent updates introduce a new constant for handling the maximum number of contacts per request and expand utility functions in the HubSpot integration. These utilities include functions for object and identifier type validation, extracting unique values, and performing searches within HubSpot. Additionally, there are updates to existing functions to incorporate these new utilities, and corresponding tests have been added or modified to ensure functionality is as expected. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
mihir-4116
changed the title
mihir-4116
changed the title
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #2907 +/- ##
========================================
Coverage 87.26% 87.27%
========================================
Files 534 534
Lines 29055 29069 +14
Branches 6918 6917 -1
========================================
+ Hits 25355 25370 +15
+ Misses 3358 3357 -1
Partials 342 342 ☔ View full report in Codecov by Sentry. |
|
Can we add unit tests for the utils.js |
|
@coderabbitai review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- src/v0/destinations/hs/config.js (2 hunks)
- src/v0/destinations/hs/util.js (6 hunks)
- src/v0/destinations/hs/util.test.js (2 hunks)
Additional comments: 11
src/v0/destinations/hs/config.js (2)
- 67-68: The addition of
MAX_CONTACTS_PER_REQUESTconstant is consistent with the PR's objective to implement data chunking based on a batch limit.- 114-114: Ensure that the new constant
MAX_CONTACTS_PER_REQUESTis being used wherever applicable to enforce the batch limit.Verification successful
I have verified the usage of the
MAX_CONTACTS_PER_REQUESTconstant in the codebase. It is being used in theconfig.jsfile where it is defined, and also in theutil.jsfile where it is utilized to chunk values. Based on this verification, it seems that the new constant is being used correctly to enforce the batch limit as intended.Conclusion
The usage of the
MAX_CONTACTS_PER_REQUESTconstant has been verified and found to be consistent with the expected usage. Therefore, the review comment has been successfully addressed.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of MAX_CONTACTS_PER_REQUEST in the codebase to ensure it's being used correctly. rg 'MAX_CONTACTS_PER_REQUEST' --type jsLength of output: 331
src/v0/destinations/hs/util.test.js (3)
- 1-5: The import changes reflect the addition of new utility functions and are consistent with the changes in
util.js.- 48-110: The tests for
getObjectAndIdentifierTypefunction are well-structured and cover both positive and negative scenarios.- 112-202: The tests for
extractUniqueValuesfunction are comprehensive and check for both non-empty and empty input scenarios.src/v0/destinations/hs/util.js (6)
- 1-1: Disabling the
no-await-in-loopeslint rule is justified in this context as the loop contains asynchronous calls that depend on the result of the previous iteration.- 474-483: The
getObjectAndIdentifierTypefunction correctly extracts theobjectTypeandidentifierTypefrom the message and throws an error if they are not found.- 490-498: The
extractUniqueValuesfunction correctly extracts and deduplicates values from the input array.- 536-625: > 📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [509-568]
The
performHubSpotSearchfunction implements pagination correctly and accumulates results from potentially multiple API calls. Ensure that the batch size and pagination are handled correctly to avoid rate limits and performance issues.
- 575-621: The
getExistingDatafunction uses the new utility functions to perform its task. Ensure that the chunking of values (lodash.chunk) respects theMAX_CONTACTS_PER_REQUESTand that the error handling is robust.- 734-735: Exporting the new utility functions aligns with the changes made in the test file and the overall PR objectives.
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/2907/test-report.html |
1 similar comment
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/2907/test-report.html |
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/2907/test-report.html |
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/2907/test-report.html |
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/2907/test-report.html |
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/2907/test-report.html |
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/2907/test-report.html |
|
Test report for this run is available at: https://test-integrations-dev.s3.amazonaws.com/integrations-test-reports/rudder-transformer/2907/test-report.html |
|
What are the changes introduced in this PR?
Write a brief explainer on your code changes.
Please explain the objectives of your changes below
Put down any required details on the broader aspect of your changes. If there are any dependent changes, mandatorily mention them here
Type of change
If the pull request is a bug-fix, enhancement or a refactor, please fill in the details on the changes made.
Existing capabilities/behavior
New capabilities/behavior
If the pull request is a new feature,
Any technical or performance related pointers to consider with the change?
N/A
Any new dependencies introduced with this change?
N/A
Any new generic utility introduced or modified. Please explain the changes.
N/A
If the PR has changes in more than 10 files, please mention why the changes were not split into multiple PRs.
N/A
If multiple linear tasks are associated with the PR changes, please elaborate on the reason:
N/A
Developer checklist
No breaking changes are being introduced.
Are all related docs linked with the PR?
Are all changes manually tested?
Does this change require any documentation changes?
Are relevant unit and component test-cases added?
Reviewer checklist
Is the type of change in the PR title appropriate as per the changes?
Verified that there are no credentials or confidential data exposed with the changes.
Summary by CodeRabbit
New Features
Bug Fixes
Tests