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

Search endpoint accounts for Search.gov constraints #2408

Merged
merged 3 commits into from
Oct 30, 2018

Conversation

hpjaj
Copy link
Contributor

@hpjaj hpjaj commented Oct 30, 2018

Description of change

  1. Load testing failed due to our staging IP address being blacklisted. Consequently, we were receiving 429 errors from Search.gov around too many requests.
  2. Since Search.gov’s offset is maxed out at 999, we cannot view all of the potential search results.

For example, if the total search results are 35k, we would only be able to view the first 1000.

The total pages and total entries now accounts for this constraint.

Testing done

Local testing

Testing planned

Load testing against the Search.gov API

Acceptance Criteria (Definition of Done)

Unique to this PR

  • Adds error handling for Search.gov rate limiting case
  • Adds swagger docs specifically for the 429 rate limiting case for the FE to consume
  • For pagination, sets the total_pages value to the total viewable pages, based on Search.gov constraints
  • For pagination, sets the total_entries value to the total viewable entries, based on Search.gov constraints
  • Spec coverage
  • Updated swagger docs
  • Yard docs

Applies to all PRs

  • Appropriate logging
  • Swagger docs have been updated, if applicable
  • Provide link to originating GitHub issue, or connected to it via ZenHub
  • Does not contain any sensitive information (i.e. PII/credentials/internal URLs/etc., in logging, hardcoded, or in specs)
  • Provide which alerts would indicate a problem with this functionality (if applicable)

Screenshot

Dedicated 429 error response

image

@hpjaj hpjaj requested review from kfrz, bshyong and narin October 30, 2018 20:43
Copy link
Contributor

@kfrz kfrz left a comment

Choose a reason for hiding this comment

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

Thanks, Harry. This is awesome! LGTM.

@va-bot va-bot temporarily deployed to search-max-pages-per-max-offset/master October 30, 2018 20:58 Inactive
end

def maximum_viewable_entries
(ENTRIES_PER_PAGE * total_viewable_pages) + (ENTRIES_PER_PAGE - 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is calculation correct? It seems that if there’s only 1 page this would return 19

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bshyong - Yeah, it would for sure; however, total_entries would return 1, and so

    def total_viewable_entries
      [total_entries, maximum_viewable_entries].min
    end

would return 1.

Since Search.gov’s offset is maxed out at 999, we cannot view all of the potential search results.

For example, if the total search results are 35k, we would only be able to view the first 1000.

The total pages and total entries now accounts for this constraint.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants