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

refactor: forester: queue fetching logic to support configurable indices #1200

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

sergeytimoshin
Copy link
Contributor

Enhanced fetch_queue_item_data to accept start index, length, and queue length parameters.

Enhanced fetch_queue_item_data to accept start index, length, and queue length parameters. Updated configuration to include queue settings and adjusted test cases accordingly.
STATE_QUEUE_LENGTH=28807
ADDRESS_QUEUE_START_INDEX=0
ADDRESS_QUEUE_LENGTH=28807
Copy link
Contributor

Choose a reason for hiding this comment

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

how easy is it to change these values?
Would it be easier to change if these are env variables?

Copy link
Contributor

@ananas-block ananas-block 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 to me in general. The only thing I am worried about is that right now it seems like we will need to recompile the forester to change the parameters. I think it would be more elegant and flexible to use env variables that we read during runtime so that we can just restart the forester dynamically with a different config in case we want to run multiple foresters on short notice.

@sergeytimoshin sergeytimoshin merged commit 38e4c46 into main Sep 11, 2024
7 checks passed
@sergeytimoshin sergeytimoshin deleted the sergey/forester-queue-limit branch September 11, 2024 10:43
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.

2 participants