-
Notifications
You must be signed in to change notification settings - Fork 27
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: Refactor and split-up settings #451
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zeeshanlakhani
approved these changes
Nov 28, 2023
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.
Great job @bgins.
zeeshanlakhani
pushed a commit
that referenced
this pull request
Nov 29, 2023
# Description This PR implements the following changes: - [x] Break up `[node.network]` settings into smaller groups - [x] Move `[monitoring]` settings to `[node.monitoring]` - [x] Add `defaults.toml` with complete settings matching the defaults - [x] Reduce the settings passed to consumers to the minimum necessary - [x] Update test fixtures using the new settings - [x] Update and add comments A complete version of the updated settings will look something like: ```toml [node] gc_interval = 1800 shutdown_timeout = 20 [node.database] url = "homestar.db" max_pool_size = 100 [node.monitoring] process_collector_interval = 5000 console_subscriber_port = 6669 [node.network] events_buffer_len = 1024 poll_cache_interval = 1000 [node.network.ipfs] host = "127.0.0.1" port = 5001 [node.network.libp2p] listen_address = "/ip4/0.0.0.0/tcp/0" node_addresses = [] announce_addresses = [] transport_connection_timeout = 60 max_connected_peers = 32 max_announce_addresses = 10 [node.network.libp2p.mdns] enable = true enable_ipv6 = false query_interval = 300 ttl = 540 [node.network.libp2p.rendezvous] enable_client = true enable_server = false registration_ttl = 7200 discovery_interval = 600 [node.network.libp2p.pubsub] enable = false duplication_cache_time = 1 heartbeat = 60 idle_timeout = 86400 max_transmit_size = 10485760 mesh_n_low = 1 mesh_n_high = 10 mesh_n = 2 mesh_outbound_min = 1 [node.network.libp2p.dht] p2p_provider_timeout = 30 receipt_quorum = 2 workflow_quorum = 3 [node.network.keypair_config] random = { } [node.network.metrics] port = 4000 [node.network.rpc] host = "::1" port = 3030 max_connections = 10 server_timeout = 120 [node.network.webserver] host = "127.0.0.1" port = 1337 timeout = 120 websocket_capacity = 2048 websocket_receiver_timeout = 30000 ``` ## Link to issue Closes #442 ## Type of change - [x] Refactor (non-breaking change that updates existing functionality) - [ ] This change requires a documentation update. (It does. Will do.) - [x] Comments have been added/updated. ## Test plan (required) All existing tests should pass with the updated fixtures.
bgins
added a commit
that referenced
this pull request
Nov 29, 2023
## Description Includes: - re-purposing of feature flags * metrics is always a thing (on) * monitoring is the gated feature * The websocket-server flag is gone, we only gate push notifications - JSON-RPC setup and RPC method register - Prometheus exposition format to JSON parser Other features and other fixes: - [x] e2e testing of run workflow - [x] #407 - [x] #410 - [x] #418 - [x] #424 - [x] #354 - [x] #409 - [x] #425 - [x] #426 - [x] #429 - [x] #433 - [x] #435 - [x] #421 - [x] #436 - [x] #437 - [x] #444 - [x] #438 - [x] #390 - [x] #451 - [x] #456 --------- Signed-off-by: Brian Ginsburg <[email protected]> Signed-off-by: Zeeshan Lakhani <[email protected]> Co-authored-by: Brian Ginsburg <[email protected]> Co-authored-by: Hugo Dias <[email protected]>
bgins
added a commit
that referenced
this pull request
Nov 29, 2023
Includes: - re-purposing of feature flags * metrics is always a thing (on) * monitoring is the gated feature * The websocket-server flag is gone, we only gate push notifications - JSON-RPC setup and RPC method register - Prometheus exposition format to JSON parser Other features and other fixes: - [x] e2e testing of run workflow - [x] #407 - [x] #410 - [x] #418 - [x] #424 - [x] #354 - [x] #409 - [x] #425 - [x] #426 - [x] #429 - [x] #433 - [x] #435 - [x] #421 - [x] #436 - [x] #437 - [x] #444 - [x] #438 - [x] #390 - [x] #451 - [x] #456 --------- Signed-off-by: Brian Ginsburg <[email protected]> Signed-off-by: Zeeshan Lakhani <[email protected]> Co-authored-by: Brian Ginsburg <[email protected]> Co-authored-by: Hugo Dias <[email protected]>
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR implements the following changes:
[node.network]
settings into smaller groups[monitoring]
settings to[node.monitoring]
defaults.toml
with complete settings matching the defaultsA complete version of the updated settings will look something like:
Link to issue
Closes #442
Type of change
Test plan (required)
All existing tests should pass with the updated fixtures.