Skip to content

TDL-16246: Make MAX_API_RESPONSE_SIZE configurable and add pagination for card stream. #30

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 18 commits into from
Sep 28, 2022
Merged

TDL-16246: Make MAX_API_RESPONSE_SIZE configurable and add pagination for card stream. #30

merged 18 commits into from
Sep 28, 2022

Conversation

savan-chovatiya
Copy link
Contributor

@savan-chovatiya savan-chovatiya commented Nov 18, 2021 •
edited by dsprayberry
Loading

Description of change

TDL-16246: Make MAX_API_RESPONSE_SIZE configurable

  • Added support of configurable parameter cards_response_size in config file for card stream.(Default : 1000)
  • Added pagination for the card stream.

Manual QA steps

  • The verified output of the old tap with data emitted with a new approach for card stream.
  • Provided limit in integer, and string format and verified that it's used in the request.

Risks

  • Minimal. Test coverage suggests no issues with pagination.

Rollback steps

  • revert this branch

dbshah1212
dbshah1212 reviewed Nov 18, 2021
View reviewed changes
dbshah1212
dbshah1212 reviewed Nov 18, 2021
View reviewed changes
dbshah1212
dbshah1212 reviewed Nov 18, 2021
View reviewed changes
dbshah1212
dbshah1212 reviewed Nov 18, 2021
View reviewed changes
Copy link

@dbshah1212 dbshah1212 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, provided some comment changes.

@savan-chovatiya savan-chovatiya changed the title TDL-16246: Added config parameter and pagination fo card stream TDL-16246: Make MAX_API_RESPONSE_SIZE configurable and add pagination for card stream. Nov 18, 2021
kspeer825
kspeer825 approved these changes Nov 19, 2021
View reviewed changes
Copy link
Contributor

@kspeer825 kspeer825 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with 1 comment.

@hpatel41 hpatel41 mentioned this pull request May 10, 2022
Open
@luandy64
Improve the unit test (#32)
e985076 
* Dry up the unittest

1. Remove unused imports
2. Add a default config that each test uses
3. Use `setUp` to create a shared variable
4. Order assertions so it's (expected, actual)
5. Simplify the mocking to just one function
6. Simplify the API for `mocked_get`

* Fix the doc strings, update the test class name
luandy64
luandy64 reviewed Sep 28, 2022
View reviewed changes
@dsprayberry dsprayberry mentioned this pull request Sep 28, 2022
Merged
@dsprayberry dsprayberry merged commit b5cd877 into master Sep 28, 2022
@dsprayberry dsprayberry deleted the TDL-16246-make-max-api-response-size-configurable-for-cards branch September 28, 2022 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kspeer825 kspeer825 kspeer825 approved these changes

@luandy64 luandy64 luandy64 approved these changes

@KrisPersonal KrisPersonal KrisPersonal approved these changes

@dbshah1212 dbshah1212 Awaiting requested review from dbshah1212

@manand31 manand31 Awaiting requested review from manand31

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@savan-chovatiya @luandy64 @dbshah1212 @kspeer825 @KrisPersonal @dsprayberry @namrata270998