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

Fix DB_RESULT_LIMIT Parsing to Allow Integer Configuration #999

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

TeachMeTW
Copy link
Contributor

@TeachMeTW TeachMeTW commented Dec 9, 2024

Summary

Fixes an issue where the DB_RESULT_LIMIT configuration was being treated as a string, preventing it from being set as an integer dynamically. The update ensures the value is properly parsed as an integer, enabling correct configuration usage.

Changes

  • Ensures DB_RESULT_LIMIT is parsed as an integer.
  • Retains the default value of 250000 if the configuration is not explicitly set.

Why This Fix?

  • Previously, the value of DB_RESULT_LIMIT was treated as a string, leading to unexpected behavior when attempting to use or override it.
  • This change resolves the type mismatch and ensures the value can be correctly parsed and used as an integer.

Testing

  • Tested with no DB_RESULT_LIMIT set: defaults to 250000.
  • Tested with DB_RESULT_LIMIT set to a valid integer: properly reflects the configured value.
  • Tested with invalid DB_RESULT_LIMIT (non-numeric string): verified that it raises an appropriate error -- defaults to 250,000 if error
image

Previously, `DB_RESULT_LIMIT` was treated as a string, preventing it from being set dynamically. This update ensures it is parsed as an integer, fixing the issue. Updated logic: `result_limit = int(config.get("DB_RESULT_LIMIT", 250000))`.
@TeachMeTW
Copy link
Contributor Author

@JGreenlee ready for review (albeit small change)

Copy link
Contributor

@JGreenlee JGreenlee left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready for review by Shankari
Development

Successfully merging this pull request may close these issues.

2 participants