-
Notifications
You must be signed in to change notification settings - Fork 65
apollo_http_server: polling interval in config #11537
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
apollo_http_server: polling interval in config #11537
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
matanl-starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matanl-starkware reviewed 5 files and all commit messages, and made 2 comments.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware).
crates/apollo_deployments/resources/app_configs/replacer_http_server_config.json line 4 at r1 (raw file):
"http_server_config.dynamic_config.accept_new_txs": true, "http_server_config.dynamic_config.max_sierra_program_size": 4194304, "http_server_config.static_config.dynamic_config_poll_interval": 1000,
What about making it a dynamic config?
Code quote:
"http_server_config.static_config.dynamic_config_poll_interval"crates/apollo_deployments/resources/app_configs/replacer_http_server_config.json line 4 at r1 (raw file):
"http_server_config.dynamic_config.accept_new_txs": true, "http_server_config.dynamic_config.max_sierra_program_size": 4194304, "http_server_config.static_config.dynamic_config_poll_interval": 1000,
Suggestion:
dynamic_config_poll_interval_millise0d6656 to
bf71e51
Compare
d3110ba to
5e4ca34
Compare
bf71e51 to
9933489
Compare
5e4ca34 to
1db5b08
Compare
9933489 to
e424ecb
Compare
1db5b08 to
3e7fc5d
Compare
3e7fc5d to
824f722
Compare
e424ecb to
533c4fd
Compare
824f722 to
3207adb
Compare
533c4fd to
78d173c
Compare
Itay-Tsabary-Starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Itay-Tsabary-Starkware made 2 comments.
Reviewable status: 4 of 5 files reviewed, all discussions resolved (waiting on @matanl-starkware).
crates/apollo_deployments/resources/app_configs/replacer_http_server_config.json line 4 at r1 (raw file):
Previously, matanl-starkware (Matan Lior) wrote…
What about making it a dynamic config?
It's a static config, affecting the parameters of the dynamic config (please see it is under static_config)
crates/apollo_deployments/resources/app_configs/replacer_http_server_config.json line 4 at r1 (raw file):
"http_server_config.dynamic_config.accept_new_txs": true, "http_server_config.dynamic_config.max_sierra_program_size": 4194304, "http_server_config.static_config.dynamic_config_poll_interval": 1000,
It's a duration object, not a number, it doesn't have units
I agree it's serialization name is awkward, but better like this imo.
Itay-Tsabary-Starkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Itay-Tsabary-Starkware reviewed 5 files and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware).
78d173c to
eac93bd
Compare
Merge activity
|
43cd44c

No description provided.