Skip to content

Conversation

@Itay-Tsabary-Starkware
Copy link
Contributor

No description provided.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Collaborator

@matanl-starkware matanl-starkware left a comment

Choose a reason for hiding this comment

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

@matanl-starkware reviewed 29 of 29 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @Itay-Tsabary-Starkware)


crates/apollo_infra/src/component_client/remote_component_client.rs line 154 at r1 (raw file):

        let mut connector = HttpConnector::new();
        connector.set_nodelay(config.set_tcp_nodelay);
        connector.set_connect_timeout(Some(Duration::from_millis(config.connection_timeout_ms)));

I'd rather avoid the conversion to Duration each time.
Is it possible to keep as Duration in the config struct?

Code quote:

Duration::from_millis(config.connection_timeout_ms)

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware changed the base branch from 11-23-apollo_infra_add_configurable_tcp_nodelay_to_infra_remote_connetions to main-v0.14.1 November 27, 2025 08:16
@Itay-Tsabary-Starkware Itay-Tsabary-Starkware force-pushed the 11-25-apollo_infra_add_config_for_max_connection_attempt_duration branch from df4c372 to 818c0d3 Compare November 27, 2025 08:16
@graphite-app
Copy link

graphite-app bot commented Nov 27, 2025

Merge activity

  • Nov 27, 8:16 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

@Itay-Tsabary-Starkware Itay-Tsabary-Starkware added this pull request to the merge queue Nov 27, 2025
Merged via the queue into main-v0.14.1 with commit 1f61e93 Nov 27, 2025
21 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants