Storage enum

[dinmukhamedm]

remove Arc dyn from Storage

• slightly optimize writing large payloads

To be reviewed after / on top of #403

---

Important

Refactor to replace Arc<dyn> with enums and traits for Storage, SemanticSearch, and CodeExecutor, improving code structure and performance.

• Refactoring:
  • Replace Arc<dyn SemanticSearch> with Arc<SemanticSearch> in semantic_search.rs, routes/datasets.rs, routes/projects.rs, and other files.
  • Replace Arc<dyn CodeExecutor> with Arc<CodeExecutor> in code_executor.rs, pipeline/context.rs, and other files.
  • Replace Arc<dyn Storage> with Arc<Storage> in storage.rs, traces/consumer.rs, and other files.
• Enums and Traits:
  • Introduce SemanticSearch, CodeExecutor, and Storage enums with variants for different implementations.
  • Define SemanticSearchTrait, CodeExecutorTrait, and StorageTrait for respective functionalities.
• Miscellaneous:
  • Minor optimizations in traces/spans.rs for handling large payloads.
  • Update imports and usage across multiple files to align with new enum and trait structures.

^{This description was created by for 68e3e48. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to 68e3e48 in 2 minutes and 24 seconds

More details

• Looked at 712 lines of code in 23 files
• Skipped 0 files when reviewing.
• Skipped posting 10 drafted comments based on config settings.

1. app-server/src/code_executor/mod.rs:15

• Draft comment:
  Good use of enum_dispatch for CodeExecutor variants. Ensure that adding new variants in the future updates the conversion implementations accordingly.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

2. app-server/src/semantic_search/mod.rs:20

• Draft comment:
  The use of enum_dispatch for SemanticSearch is clear. Confirm that the selected variant (Grpc or Mock) is correctly configured in production environments.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

3. app-server/src/storage/mod.rs:25

• Draft comment:
  Consider using floating point arithmetic in the payload size threshold calculation; integer division in (7 / 2) gives 3 (3 * 128_000 = 384000) which contradicts the comment expecting approx 448KB. Use something like ((7.0/2.0)*128000.0) as usize.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

4. app-server/src/traces/spans.rs:668

• Draft comment:
  In the 'store_payloads' function, ensure proper error handling when serializing payloads. The use of .unwrap() in serde_json::to_value can panic if serialization fails.
• Reason this comment was not posted:
  Comment was on unchanged code.

5. app-server/src/main.rs:342

• Draft comment:
  When constructing 'semantic_search' and 'code_executor', use of .into() is clear but make sure the conversion implementations for the new enum types handle all edge cases.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50%
  None

6. app-server/src/pipeline/nodes/semantic_similarity.rs:63

• Draft comment:
  Potential panic: Using unwrap() on response.scores.get(0) may cause a runtime error if the scores vector is empty. Consider adding a check or handling the empty case gracefully.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

7. app-server/src/storage/s3.rs:15

• Draft comment:
  The get_url() function assumes the key always starts with 'project/' and contains two segments. If key format deviates, unwrap() may panic. Consider adding error handling for unexpected key formats.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

8. app-server/src/traces/spans.rs:690

• Draft comment:
  When serializing the input to data and truncating to 100 characters using String::from_utf8_lossy() and .chars().take(100), ensure this truncation meets your requirements. Losing context might affect debugging. Consider logging full data in case of errors.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 40% <= threshold 50%
  The comment suggests ensuring that truncation meets requirements and considers logging full data for debugging. It indirectly asks the author to ensure behavior is intended, which violates the rules. However, it also provides a suggestion to log full data, which could be useful.

9. app-server/src/semantic_search/semantic_search_impl.rs:51

• Draft comment:
  The model is hard-coded as Model::CohereMultilingual. If supporting multiple models is desired, consider making the model configurable.
• Reason this comment was not posted:
  Confidence changes required: 50% <= threshold 50%
  None

10. app-server/src/main.rs:258

• Draft comment:
  Converting storage instances with .into() for both S3Storage and MockStorage is effective; ensure that the conversion implementations for these types are correctly defined without unintended side effects.
• Reason this comment was not posted:
  Confidence changes required: 30% <= threshold 50%
  None

Workflow ID: wflow_s9hhpZZlB6AFxVZf

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[dinmukhamedm]

Is already part of #405