Skip to content
This repository has been archived by the owner on Nov 30, 2022. It is now read-only.

Updating offset pagination strategy to parse numeric strings as integers #1364

Merged
merged 2 commits into from
Sep 21, 2022

Conversation

galvana
Copy link
Collaborator

@galvana galvana commented Sep 21, 2022

Purpose

ConnectionConfig secret values might be stored as strings, but when these values are used in the offset pagination strategy, they need to be converted back to integers. This update casts numeric strings to integers or throws a validation error if the string cannot be cast to an integer.

Changes

  • Added str to int conversion in the offset pagination strategy and added the appropriate tests

Checklist

  • Update CHANGELOG.md file
    • Merge in main so the most recent CHANGELOG.md file is being appended to
    • Add description within the Unreleased section in an appropriate category. Add a new category from the list at the top of the file if the needed one isn't already there.
  • Good unit test/integration test coverage
  • The Run Unsafe PR Checks label has been applied, and checks have passed, if this PR touches any external services

@galvana galvana added the run unsafe ci checks Triggers running of unsafe CI checks label Sep 21, 2022
@galvana galvana requested a review from adamsachs September 21, 2022 17:02
Copy link
Contributor

@adamsachs adamsachs left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@eastandwestwind eastandwestwind 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, thanks for the thorough tests as well @galvana !

@galvana galvana merged commit 5ecab66 into main Sep 21, 2022
@galvana galvana deleted the offset-pagination-limit branch September 21, 2022 17:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
run unsafe ci checks Triggers running of unsafe CI checks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants