-
-
Notifications
You must be signed in to change notification settings - Fork 158
fix multiple versions of rustls #1513
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
Conversation
WalkthroughUpdated dependencies (rustls 0.23, http 1.0, reqwest 0.12). Replaced many Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ActixServer
participant Downstream as ReqwestClient
Note over ActixServer: Actix request handling
Client->>ActixServer: HTTP request (headers + body)
ActixServer->>ActixServer: Parse & validate headers
ActixServer->>ActixServer: Convert headers (guarded)
alt header conversion OK
ActixServer->>Downstream: Forward request (reqwest HeaderMap populated)
else invalid headers skipped
Note right of ActixServer: Skip invalid header(s)
ActixServer->>Downstream: Forward request (partial headers)
end
Downstream-->>ActixServer: Response (status, headers, body)
ActixServer-->>Client: Proxy response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 4❌ Failed checks (3 warnings, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/main.rs:
- Around line 36-41: The current installation of the rustls crypto provider
(rustls::crypto::ring::default_provider().install_default()) only logs errors
and lets the app continue, which can hide real failures; update the handling to
match on the returned Err, distinguish the "already installed" case from other
errors by inspecting the error variant or message, log a clear message for the
benign "already installed" case and call std::process::exit(1) (or return an Err
from main) for any other fatal error so TLS operations cannot proceed in a
broken state.
🧹 Nitpick comments (1)
src/handlers/http/cluster/mod.rs (1)
371-380: Consider logging failed header conversions for better observability.The safe header conversion pattern correctly handles incompatible headers by skipping them. However, silently dropping headers could make debugging difficult if critical headers fail to convert.
🔍 Add logging for failed conversions
for (key, value) in headers.iter() { // Convert actix header name/value to reqwest header name/value if let Ok(name) = reqwest::header::HeaderName::from_bytes(key.as_str().as_bytes()) && let Ok(val) = reqwest::header::HeaderValue::from_bytes(value.as_bytes()) { reqwest_headers.insert(name, val); + } else { + warn!("Failed to convert header '{}' for ingestor sync", key); } }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
Cargo.tomlsrc/alerts/mod.rssrc/correlation.rssrc/handlers/http/cluster/mod.rssrc/handlers/http/cluster/utils.rssrc/handlers/http/health_check.rssrc/handlers/http/ingest.rssrc/handlers/http/llm.rssrc/handlers/http/logstream.rssrc/handlers/http/mod.rssrc/handlers/http/modal/ingest/ingestor_logstream.rssrc/handlers/http/modal/ingest/ingestor_rbac.rssrc/handlers/http/modal/mod.rssrc/handlers/http/modal/query/querier_logstream.rssrc/handlers/http/modal/ssl_acceptor.rssrc/handlers/http/modal/utils/ingest_utils.rssrc/handlers/http/oidc.rssrc/handlers/http/query.rssrc/handlers/http/rbac.rssrc/handlers/http/role.rssrc/handlers/http/users/dashboards.rssrc/handlers/http/users/filters.rssrc/main.rssrc/metastore/metastores/object_store_metastore.rssrc/metastore/mod.rssrc/metrics/mod.rssrc/metrics/prom_utils.rssrc/parseable/mod.rssrc/prism/home/mod.rssrc/prism/logstream/mod.rssrc/utils/error.rssrc/utils/header_parsing.rs
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-10-20T17:48:53.444Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1448
File: src/handlers/http/cluster/mod.rs:1370-1400
Timestamp: 2025-10-20T17:48:53.444Z
Learning: In src/handlers/http/cluster/mod.rs, the billing metrics processing logic should NOT accumulate counter values from multiple Prometheus samples with the same labels. The intended behavior is to convert each received counter from nodes into individual events for ingestion, using `.insert()` to store the counter value directly.
Applied to files:
src/metrics/prom_utils.rssrc/handlers/http/cluster/mod.rs
📚 Learning: 2025-05-01T10:27:56.858Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1305
File: src/handlers/http/users/dashboards.rs:0-0
Timestamp: 2025-05-01T10:27:56.858Z
Learning: The `add_tile()` function in `src/handlers/http/users/dashboards.rs` should use `get_dashboard_by_user(dashboard_id, &user_id)` instead of `get_dashboard(dashboard_id)` to ensure proper authorization checks when modifying a dashboard.
Applied to files:
src/handlers/http/users/dashboards.rs
📚 Learning: 2025-09-18T09:52:07.554Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1415
File: src/storage/object_storage.rs:173-177
Timestamp: 2025-09-18T09:52:07.554Z
Learning: In Parseable's upload system (src/storage/object_storage.rs), the update_storage_metrics function can safely use path.metadata().map_err() to fail on local file metadata read failures because parquet validation (validate_uploaded_parquet_file) ensures file integrity before this step, and the system guarantees local staging files remain accessible throughout the upload flow.
Applied to files:
src/metastore/mod.rs
📚 Learning: 2025-09-06T04:26:17.191Z
Learnt from: parmesant
Repo: parseablehq/parseable PR: 1424
File: src/enterprise/utils.rs:65-72
Timestamp: 2025-09-06T04:26:17.191Z
Learning: In Parseable's metastore implementation, MetastoreError::to_detail() returns a MetastoreErrorDetail struct (not a string), which contains structured error information including operation, message, stream_name, and other contextual fields. This struct is designed to be boxed in ObjectStorageError::MetastoreError(Box<MetastoreErrorDetail>).
Applied to files:
src/metastore/mod.rs
📚 Learning: 2025-08-18T14:56:18.463Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1405
File: src/storage/object_storage.rs:997-1040
Timestamp: 2025-08-18T14:56:18.463Z
Learning: In Parseable's staging upload system (src/storage/object_storage.rs), failed parquet file uploads should remain in the staging directory for retry in the next sync cycle, while successful uploads remove their staged files immediately. Early return on first error in collect_upload_results is correct behavior as concurrent tasks handle their own cleanup and failed files need to stay for retry.
Applied to files:
src/metastore/mod.rssrc/parseable/mod.rs
📚 Learning: 2025-06-16T04:56:21.613Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1346
File: src/sync.rs:78-83
Timestamp: 2025-06-16T04:56:21.613Z
Learning: In Rust async code, `#[tokio::main]` is appropriate when functions are called from separate OS threads (like `thread::spawn()`), as it creates a new Tokio runtime for that thread. The "Cannot start a runtime from within a runtime" error only occurs when `#[tokio::main]` functions are called from within an existing Tokio runtime context.
Applied to files:
src/main.rs
📚 Learning: 2025-10-21T02:22:24.403Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1448
File: src/parseable/mod.rs:419-432
Timestamp: 2025-10-21T02:22:24.403Z
Learning: In Parseable's internal stream creation (`create_internal_stream_if_not_exists` in `src/parseable/mod.rs`), errors should not propagate to fail server initialization. The function creates both pmeta and pbilling internal streams, and failures are logged but the function always returns `Ok(())` to ensure server startup resilience. Individual stream creation failures should not prevent syncing of successfully created streams.
Applied to files:
src/handlers/http/logstream.rs
📚 Learning: 2025-09-05T09:18:44.813Z
Learnt from: parmesant
Repo: parseablehq/parseable PR: 1425
File: src/query/mod.rs:484-495
Timestamp: 2025-09-05T09:18:44.813Z
Learning: In the Parseable system, stream names and column names cannot contain quotes, which eliminates SQL injection concerns when interpolating these identifiers directly into SQL queries in src/query/mod.rs.
Applied to files:
src/alerts/mod.rs
🧬 Code graph analysis (8)
src/handlers/http/oidc.rs (8)
src/handlers/http/ingest.rs (1)
status_code(511-543)src/handlers/http/rbac.rs (1)
status_code(419-439)src/handlers/http/users/dashboards.rs (1)
status_code(264-275)src/metastore/mod.rs (1)
status_code(149-159)src/prism/home/mod.rs (1)
status_code(484-494)src/prism/logstream/mod.rs (1)
status_code(391-404)src/utils/error.rs (1)
status_code(33-35)src/utils/header_parsing.rs (1)
status_code(41-43)
src/handlers/http/ingest.rs (16)
src/alerts/mod.rs (1)
status_code(991-1017)src/correlation.rs (1)
status_code(338-350)src/handlers/http/llm.rs (1)
status_code(150-157)src/handlers/http/logstream.rs (1)
status_code(592-628)src/handlers/http/oidc.rs (1)
status_code(566-573)src/handlers/http/query.rs (1)
status_code(593-599)src/handlers/http/rbac.rs (1)
status_code(419-439)src/handlers/http/role.rs (1)
status_code(179-188)src/handlers/http/users/dashboards.rs (1)
status_code(264-275)src/handlers/http/users/filters.rs (1)
status_code(146-155)src/metastore/mod.rs (1)
status_code(149-159)src/metrics/mod.rs (1)
status_code(785-789)src/prism/home/mod.rs (1)
status_code(484-494)src/prism/logstream/mod.rs (1)
status_code(391-404)src/utils/error.rs (1)
status_code(33-35)src/utils/header_parsing.rs (1)
status_code(41-43)
src/handlers/http/role.rs (5)
src/handlers/http/oidc.rs (1)
status_code(566-573)src/handlers/http/query.rs (1)
status_code(593-599)src/metastore/mod.rs (1)
status_code(149-159)src/prism/home/mod.rs (1)
status_code(484-494)src/utils/header_parsing.rs (1)
status_code(41-43)
src/handlers/http/llm.rs (15)
src/alerts/mod.rs (1)
status_code(991-1017)src/correlation.rs (1)
status_code(338-350)src/handlers/http/ingest.rs (1)
status_code(511-543)src/handlers/http/oidc.rs (1)
status_code(566-573)src/handlers/http/query.rs (1)
status_code(593-599)src/handlers/http/rbac.rs (1)
status_code(419-439)src/handlers/http/role.rs (1)
status_code(179-188)src/handlers/http/users/dashboards.rs (1)
status_code(264-275)src/handlers/http/users/filters.rs (1)
status_code(146-155)src/metastore/mod.rs (1)
status_code(149-159)src/metrics/mod.rs (1)
status_code(785-789)src/prism/home/mod.rs (1)
status_code(484-494)src/prism/logstream/mod.rs (1)
status_code(391-404)src/utils/error.rs (1)
status_code(33-35)src/utils/header_parsing.rs (1)
status_code(41-43)
src/prism/logstream/mod.rs (16)
src/alerts/mod.rs (1)
status_code(991-1017)src/correlation.rs (1)
status_code(338-350)src/handlers/http/ingest.rs (1)
status_code(511-543)src/handlers/http/llm.rs (1)
status_code(150-157)src/handlers/http/logstream.rs (1)
status_code(592-628)src/handlers/http/oidc.rs (1)
status_code(566-573)src/handlers/http/query.rs (1)
status_code(593-599)src/handlers/http/rbac.rs (1)
status_code(419-439)src/handlers/http/role.rs (1)
status_code(179-188)src/handlers/http/users/dashboards.rs (1)
status_code(264-275)src/handlers/http/users/filters.rs (1)
status_code(146-155)src/metastore/mod.rs (1)
status_code(149-159)src/metrics/mod.rs (1)
status_code(785-789)src/prism/home/mod.rs (1)
status_code(484-494)src/utils/error.rs (1)
status_code(33-35)src/utils/header_parsing.rs (1)
status_code(41-43)
src/handlers/http/query.rs (15)
src/alerts/mod.rs (1)
status_code(991-1017)src/correlation.rs (1)
status_code(338-350)src/handlers/http/ingest.rs (1)
status_code(511-543)src/handlers/http/llm.rs (1)
status_code(150-157)src/handlers/http/oidc.rs (1)
status_code(566-573)src/handlers/http/rbac.rs (1)
status_code(419-439)src/handlers/http/role.rs (1)
status_code(179-188)src/handlers/http/users/dashboards.rs (1)
status_code(264-275)src/handlers/http/users/filters.rs (1)
status_code(146-155)src/metastore/mod.rs (1)
status_code(149-159)src/metrics/mod.rs (1)
status_code(785-789)src/prism/home/mod.rs (1)
status_code(484-494)src/prism/logstream/mod.rs (1)
status_code(391-404)src/utils/error.rs (1)
status_code(33-35)src/utils/header_parsing.rs (1)
status_code(41-43)
src/metrics/mod.rs (3)
src/metastore/mod.rs (1)
status_code(149-159)src/utils/error.rs (1)
status_code(33-35)src/utils/header_parsing.rs (1)
status_code(41-43)
src/handlers/http/logstream.rs (5)
src/handlers/http/query.rs (1)
status_code(593-599)src/metastore/mod.rs (1)
status_code(149-159)src/prism/logstream/mod.rs (1)
status_code(391-404)src/utils/error.rs (1)
status_code(33-35)src/utils/header_parsing.rs (1)
status_code(41-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: coverage
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
🔇 Additional comments (34)
src/handlers/http/modal/utils/ingest_utils.rs (1)
20-20: LGTM! Import path correctly updated to use actix-web's re-export.This change aligns with the dependency upgrade strategy, consolidating imports to use actix_web's HTTP module re-exports instead of directly importing from the
httpcrate. This helps prevent version conflicts when multiple versions of rustls/http dependencies are present in the dependency tree.src/metrics/prom_utils.rs (1)
27-27: LGTM! Import path updated to usehttp::headerdirectly.The change correctly moves the header import from
actix_web::http::headertohttp::header, aligning with the dependency upgrade to http 1.0. The usage at lines 242-243 remains unchanged.src/handlers/http/modal/ingest/ingestor_logstream.rs (1)
21-21: LGTM! Import path updated to useactix_web::http::StatusCode.The change correctly moves the StatusCode import to actix_web's http module, which is the appropriate source for framework-specific HTTP status handling. Usage at lines 61 and 86 remains consistent.
src/handlers/http/cluster/utils.rs (1)
25-25: LGTM! Import path updated to usehttp::headerdirectly.Consistent with the pattern in other files, moving header utilities to the http crate directly. The usage at line 199 remains unchanged.
src/handlers/http/health_check.rs (1)
21-21: LGTM! Import path updated to useactix_web::http::StatusCode.The change aligns with the systematic migration to use actix_web's StatusCode type. Usage at lines 40, 121, and 123 remains consistent with the framework's response handling.
src/handlers/http/modal/query/querier_logstream.rs (1)
22-22: LGTM! Import path updated to useactix_web::http::StatusCode.The change is correct and consistent with the codebase. All StatusCode imports across the codebase use
actix_web::http::StatusCode, and the file correctly uses this import at lines 111, 134, and 136.Note: The header import verification in the original comment had incorrect expectations. The codebase consistently uses
actix_web::http::headerfor all header imports (found in 10+ files), nothttp::headeras suggested. All imports follow a uniform pattern of sourcing fromactix_web::http::*.Likely an incorrect or invalid review comment.
src/handlers/http/mod.rs (1)
21-21: LGTM! Import migration aligned with dependency upgrade.The StatusCode import has been correctly updated to use actix-web's http module, consistent with the http crate upgrade to 1.0 and the broader migration across the codebase.
src/handlers/http/modal/ingest/ingestor_rbac.rs (1)
21-21: LGTM! Consistent import migration.The StatusCode import has been correctly migrated to actix-web's http module, consistent with the dependency upgrade.
src/parseable/mod.rs (1)
28-29: LGTM! Import migration completed successfully.The StatusCode and header-related imports have been correctly migrated from the http crate to actix-web's http module. This change is consistent with the http 1.0 upgrade and aligns with the broader migration pattern across the codebase.
Cargo.toml (1)
34-66: Major version upgrades properly handled — no additional action needed.The PR upgrades several key dependencies with major version changes (http 0.2.7 → 1.0, rustls 0.22.4 → 0.23, reqwest 0.11.27 → 0.12). The critical breaking changes are properly addressed:
- rustls 0.23 crypto provider: Correctly initialized via
rustls::crypto::ring::default_provider().install_default()in main.rs (lines 36-41) before any TLS operations, with proper error handling.- reqwest 0.12 TLS: Explicitly enables
"rustls-tls"feature, meeting the requirement that TLS backends must be explicitly configured.- http 1.0 API compatibility: No deprecated or problematic API patterns detected; actix-web 4.9.0 ensures header/request/response types are compatible.
src/handlers/http/users/filters.rs (1)
27-27: LGTM! StatusCode import updated correctly.The import path change from
http::StatusCodetoactix_web::http::StatusCodealigns with the dependency upgrade objectives. TheResponseErrorimplementation continues to work correctly with the new import.Also applies to: 146-146
src/handlers/http/cluster/mod.rs (1)
29-30: LGTM! Import changes align with the dependency upgrade.The import restructuring correctly separates actix-web's HTTP types from the standalone
httpcrate, consistent with upgrading to reqwest 0.12 and the new rustls-based stack.Also applies to: 34-34
src/utils/error.rs (1)
19-19: LGTM! Import path updated correctly.The StatusCode import change is consistent with the dependency upgrade to align with actix-web's HTTP module.
src/alerts/mod.rs (1)
19-19: LGTM! Import path updated correctly.The StatusCode import change aligns with the actix-web HTTP module migration across the codebase.
src/metastore/metastores/object_store_metastore.rs (1)
24-24: LGTM! Import path updated correctly.The StatusCode import change is consistent with the dependency upgrade and maintains compatibility with existing error handling throughout the file.
src/metastore/mod.rs (1)
19-19: LGTM! StatusCode import migrated to actix-web.The import change from
http::StatusCodetoactix_web::http::StatusCodealigns with the dependency upgrade objective and maintains consistent error handling.src/handlers/http/oidc.rs (2)
21-21: LGTM! Import updated for StatusCode migration.The import change from
http::StatusCodetoactix_web::http::StatusCodeis consistent with the PR-wide migration pattern.
566-573: LGTM! ResponseError implementation updated correctly.The
status_code()method now returnsStatusCodefromactix_web::http, maintaining the same error-to-status mappings as before.src/handlers/http/ingest.rs (2)
21-21: LGTM! Import updated for StatusCode migration.The import change aligns with the dependency upgrade to use
actix_web::http::StatusCode.
511-543: LGTM! ResponseError implementation updated correctly.The
status_code()method now returnsStatusCodefromactix_web::http. The delegation toMetastoreError::status_code()at line 541 works correctly sinceMetastoreErrorwas also updated in this PR.src/handlers/http/role.rs (2)
21-21: LGTM! Import updated for StatusCode migration.The import change from
http::StatusCodetoactix_web::http::StatusCodeis consistent with the dependency upgrade.
179-188: LGTM! ResponseError implementation updated correctly.The
status_code()method returnsStatusCodefromactix_web::http, maintaining consistent error mappings. Migration verification confirms no remaining directhttp::StatusCodeimports across the codebase.src/handlers/http/modal/mod.rs (1)
153-153: TLS configuration is properly compatible with rustls 0.23. Thessl_acceptormodule correctly uses the rustls 0.23 API with the builder-with-provider pattern (ServerConfig::builder_with_provider), custom provider setup, and the appropriatewith_safe_default_protocol_versions()method. The configuration returned is fully compatible with thebind_rustls_0_23method call.src/correlation.rs (1)
21-21: LGTM! StatusCode import migration is correct.The migration from
http::StatusCodetoactix_web::http::StatusCodeis consistent with the broader upgrade to consolidate on a single HTTP types source, eliminating the rustls version conflicts mentioned in the PR objectives.Also applies to: 338-338
src/prism/logstream/mod.rs (1)
21-21: LGTM! Consistent StatusCode migration.The import and return type changes align with the codebase-wide migration to
actix_web::http::StatusCode.Also applies to: 391-391
src/utils/header_parsing.rs (1)
19-19: LGTM! StatusCode migration is correct.The import and usage changes are consistent with the project-wide migration to
actix_web::http::StatusCode.Also applies to: 41-42
src/handlers/http/users/dashboards.rs (1)
28-28: LGTM! StatusCode migration is correct.The import and return type changes align with the codebase-wide consolidation to
actix_web::http::StatusCode.Also applies to: 264-264
src/handlers/http/modal/ssl_acceptor.rs (1)
35-38: Rustls 0.23 migration verified as complete and correct.All three verification points are confirmed:
- ✓ Crypto provider is installed globally via
install_default()in src/main.rs:38 (line 38) with proper error handling and explanatory comments- ✓ All rustls usage has been updated to the 0.23 provider-based API; no old
ServerConfig::builder()patterns remain in the codebase- ✓ The
ringfeature is enabled for rustls 0.23 in Cargo.toml with["ring", "std"]The code at lines 35-38 correctly uses
builder_with_provider()with safe default protocol versions and no client authentication. The migration is complete.src/prism/home/mod.rs (1)
21-21: LGTM! Import migration aligns with the dependency unification.The switch from
http::StatusCodetoactix_web::http::StatusCodeis consistent with the PR's goal of resolving multiple rustls versions by consolidating the HTTP type dependencies.Also applies to: 484-484
src/handlers/http/rbac.rs (1)
32-32: LGTM! StatusCode import aligned with actix-web's HTTP module.The change correctly updates the import path while preserving all existing status code mappings for the
RBACErrorvariants.Also applies to: 419-419
src/handlers/http/query.rs (1)
25-25: LGTM! Import migration is consistent with the codebase-wide refactor.The
QueryErrorResponseError implementation correctly uses the updatedStatusCodetype fromactix_web::http.Also applies to: 593-593
src/handlers/http/llm.rs (1)
19-21: LGTM! Correct separation of StatusCode and header imports.The
StatusCodeis now sourced fromactix_web::http, whilehttp::headeris retained for the reqwest client's header constant on line 109. This is the correct approach since reqwest expectshttpcrate types for request building.Also applies to: 150-150
src/handlers/http/logstream.rs (1)
509-509: LGTM! Safe status code conversion for network errors.The handling of
StreamError::Networkcorrectly converts reqwest'sStatusCode(from thehttpcrate) to actix-web'sStatusCodeusingfrom_u16(s.as_u16()). This pattern safely bridges the two types while properly falling back toINTERNAL_SERVER_ERRORif conversion fails.Also applies to: 592-592, 618-621
src/metrics/mod.rs (1)
762-765: LGTM! Consistent StatusCode migration in metrics module.The
MetricsErrortype and itsResponseErrorimplementation correctly useactix_web::http::StatusCode. The fully qualified path at line 764 is acceptable since the usage is in the parent module scope.Also applies to: 775-775, 785-785
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/main.rs:
- Around line 36-41: Move the rustls provider installation to after
init_logger() so logs are visible, i.e., call
rustls::crypto::ring::default_provider().install_default() only once the logger
is initialized (after init_logger()). Replace the blind warn! with explicit
handling: match the install_default() result and treat the "already installed"
case as benign (log at debug/info) while treating other Err variants as real
failures (log error via error! with the error details and terminate startup via
std::process::exit(1)); if the exact "already installed" variant/type isn’t
accessible, fall back to checking the error string for an "already installed"
marker but prefer matching the concrete error variant from install_default().
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main.rs
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-16T04:56:21.613Z
Learnt from: nikhilsinhaparseable
Repo: parseablehq/parseable PR: 1346
File: src/sync.rs:78-83
Timestamp: 2025-06-16T04:56:21.613Z
Learning: In Rust async code, `#[tokio::main]` is appropriate when functions are called from separate OS threads (like `thread::spawn()`), as it creates a new Tokio runtime for that thread. The "Cannot start a runtime from within a runtime" error only occurs when `#[tokio::main]` functions are called from within an existing Tokio runtime context.
Applied to files:
src/main.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Quest Smoke and Load Tests for Standalone deployments
- GitHub Check: Quest Smoke and Load Tests for Distributed deployments
- GitHub Check: Build Default x86_64-pc-windows-msvc
- GitHub Check: coverage
- GitHub Check: Build Default aarch64-unknown-linux-gnu
- GitHub Check: Build Default x86_64-unknown-linux-gnu
- GitHub Check: Build Default aarch64-apple-darwin
- GitHub Check: Build Kafka aarch64-apple-darwin
- GitHub Check: Build Kafka x86_64-unknown-linux-gnu
Fixes: #1509
Summary by CodeRabbit
Chores
Bug Fixes
Stability
✏️ Tip: You can customize this high-level summary in your review settings.