-
Notifications
You must be signed in to change notification settings - Fork 48
chore(apollo_deployments): unify deployment functions #6363
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
base: 05-07-chore_apollo_deployments_remove_consts_from_instance_config_overrides
Are you sure you want to change the base?
Conversation
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
1ad1b6d
to
e7ba6f8
Compare
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.
Reviewed 3 of 5 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Itay-Tsabary-Starkware)
crates/apollo_deployments/src/deployment_definitions/sepolia_integration.rs
line 23 at r2 (raw file):
); const FIRST_NODE_NAMESPACE: &str = "apollo-sepolia-integration-0";
Should you mention "sepolia" somewhere?
I find it confusing that in all modules it has the same name "FIRST_NODE_NAMESPACE".
crates/apollo_deployments/src/deployment.rs
line 281 at r2 (raw file):
namespace: &'static str, ) -> InstanceConfigOverride { let accepted_node_id_range = 1..=MAX_NODE_ID;
Seems like overkill.
What about checking that id >= 1 && id <= MAX_NODE_ID?
crates/apollo_deployments/src/deployment.rs
line 299 at r2 (raw file):
const MEMPOOL_SERVICE_PORT: u16 = 53200; if id == 1 {
Consider changing the id to be zero based, therefore compatible with "node_0" semantic.
In the secret_key you can encode "id+1" for simplicity.
crates/apollo_deployments/src/deployment.rs
line 303 at r2 (raw file):
"", true, "0x0101010101010101010101010101010101010101010101010101010101010101",
Consider extracting the secret_key formatting outside the if/else and using it in 4 places.
e7ba6f8
to
cf21247
Compare
d8ea152
to
0e9e401
Compare
0e9e401
to
38d8e41
Compare
cf21247
to
223d97b
Compare
38d8e41
to
7f295f6
Compare
No description provided.