Skip to content
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

modify rust package structure #2586

Merged
merged 3 commits into from
Aug 30, 2024

Conversation

kmrmt
Copy link
Contributor

@kmrmt kmrmt commented Aug 20, 2024

Description

SSIA

Related Issue

Versions

  • Vald Version: v1.7.13
  • Go Version: v1.22.6
  • Rust Version: v1.80.0
  • Docker Version: v27.1.1
  • Kubernetes Version: v1.30.3
  • Helm Version: v3.15.3
  • NGT Version: v2.2.4
  • Faiss Version: v1.8.0

Checklist

Special notes for your reviewer

Summary by CodeRabbit

  • New Features

    • Introduced a new Rust package named "algorithm" with an addition function for basic arithmetic operations.
    • Added a new Rust package named "faiss" with initial configuration for future development.
    • Introduced a new Rust package named "ngt" with updated dependencies.
  • Bug Fixes

    • Upgraded dependencies in various packages to improve performance, security, and compatibility.
  • Chores

    • Restructured the project workspace to shift focus towards new algorithm libraries, streamlining development.

Copy link
Contributor

coderabbitai bot commented Aug 20, 2024

Walkthrough

Walkthrough

The changes reflect a significant restructuring in a Rust project, with modifications to the workspace configuration and updates across various libraries. Key changes include removing older libraries, introducing new algorithm libraries, and upgrading several dependencies to newer versions. New packages such as "algorithm," "faiss," and a renamed "ngt" show a shift in organization, alongside enhancements to functionality, including basic arithmetic operations.

Changes

Files Change Summary
rust/Cargo.toml Removed workspace members "libs/ngt" and "libs/ngt-rs"; added "libs/algorithm", "libs/algorithms/ngt", and "libs/algorithms/faiss".
rust/bin/agent/Cargo.toml Replaced ngt dependency with algorithm; updated prost, tokio, and tonic to newer versions.
rust/libs/algorithm/Cargo.toml New package created for "algorithm"; specified dependencies for faiss and ngt.
rust/libs/algorithm/src/lib.rs Introduced a new public function add for adding two u64 integers, along with a unit test.
rust/libs/algorithms/faiss/Cargo.toml New package named "faiss" created with initial version "0.1.0".
rust/libs/algorithms/faiss/src/lib.rs Added a public function add similar to the one in the algorithm package.
rust/libs/algorithms/ngt/Cargo.toml Renamed package from "ngt-rs" to "ngt"; updated dependencies' versions.
rust/libs/algorithms/ngt/src/input.cpp Updated include directives to reflect new directory structure.
rust/libs/algorithms/ngt/src/lib.rs Modified the path for an included header file in the ffi module to reflect new organization.
rust/libs/observability/Cargo.toml Updated tokio and serde_json dependencies to newer versions.
rust/libs/proto/Cargo.toml Updated versions of prost, tonic, and tonic-types.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant AlgorithmLib as AL
    participant FaissLib as FL

    User->>AL: Call add(2, 2)
    AL->>AL: Perform addition
    AL-->>User: Return 4

    User->>FL: Call add(2, 2)
    FL->>FL: Perform addition
    FL-->>User: Return 4
Loading

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@vdaas-ci
Copy link
Collaborator

[CHATOPS:HELP] ChatOps commands.

  • 🙆‍♀️ /approve - approve
  • 🍱 /format - format codes and add licenses
  • /gen-test - generate test codes
  • 🏷️ /label - add labels
  • 🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

Copy link

cloudflare-workers-and-pages bot commented Aug 20, 2024

Deploying vald with  Cloudflare Pages  Cloudflare Pages

Latest commit: a6ef326
Status: ✅  Deploy successful!
Preview URL: https://b996b83a.vald.pages.dev
Branch Preview URL: https://feature-agent-rust-modify-di.vald.pages.dev

View logs

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 145b9cc and 833548a.

Files ignored due to path filters (1)
  • rust/Cargo.lock is excluded by !**/*.lock
Files selected for processing (11)
  • rust/Cargo.toml (1 hunks)
  • rust/bin/agent/Cargo.toml (1 hunks)
  • rust/libs/algorithm/Cargo.toml (1 hunks)
  • rust/libs/algorithm/src/lib.rs (1 hunks)
  • rust/libs/algorithms/faiss/Cargo.toml (1 hunks)
  • rust/libs/algorithms/faiss/src/lib.rs (1 hunks)
  • rust/libs/algorithms/ngt/Cargo.toml (1 hunks)
  • rust/libs/algorithms/ngt/src/input.cpp (1 hunks)
  • rust/libs/algorithms/ngt/src/lib.rs (1 hunks)
  • rust/libs/observability/Cargo.toml (1 hunks)
  • rust/libs/proto/Cargo.toml (1 hunks)
Files skipped from review due to trivial changes (6)
  • rust/libs/algorithm/Cargo.toml
  • rust/libs/algorithm/src/lib.rs
  • rust/libs/algorithms/faiss/Cargo.toml
  • rust/libs/algorithms/ngt/Cargo.toml
  • rust/libs/algorithms/ngt/src/input.cpp
  • rust/libs/algorithms/ngt/src/lib.rs
Additional comments not posted (7)
rust/libs/algorithms/faiss/src/lib.rs (2)

1-3: LGTM!

The add function is correctly implemented for adding two u64 integers.


5-14: Test module is well-structured.

The test module correctly tests the add function. Consider adding more test cases to cover edge cases, such as adding zero.

rust/Cargo.toml (1)

21-23: Workspace restructuring looks good.

The addition of new algorithm libraries and removal of old ones aligns with the project's restructuring goals.

Verify that the removed libraries are no longer in use within the codebase.

Verification successful

Removed libraries are no longer in use.

The search across the entire codebase confirmed that the libraries libs/ngt and libs/ngt-rs are not referenced anywhere, indicating successful removal.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the removed libraries are not in use.

# Test: Search for references to the removed libraries. Expect: No occurrences.
rg --type rust 'libs/ngt|libs/ngt-rs'

Length of output: 39


Script:

#!/bin/bash
# Description: Verify that the removed libraries are not in use anywhere in the project.

# Search for references to the removed libraries across all file types. Expect: No occurrences.
rg 'libs/ngt|libs/ngt-rs'

Length of output: 27

rust/libs/proto/Cargo.toml (1)

25-27: Dependency updates are approved.

The updates to prost, tonic, and tonic-types are reasonable for leveraging new features or improvements.

Verify that these updates are compatible with the rest of the codebase.

rust/bin/agent/Cargo.toml (1)

24-29: Dependency changes are approved.

The replacement of ngt with algorithm and updates to prost, tokio, and tonic are consistent with the project's restructuring goals.

Verify that these changes do not introduce breaking changes or issues in the codebase.

rust/libs/observability/Cargo.toml (2)

27-27: Review the impact of updating tokio to version 1.39.3.

The upgrade from tokio version 1.38.0 to 1.39.3 may introduce changes that affect asynchronous programming. Ensure that the new version is compatible with other dependencies and does not introduce breaking changes in the codebase.

Verification successful

Tokio version update appears compatible, but verify changelog for breaking changes.

The upgrade to tokio version 1.39.3 does not show immediate compatibility issues in the codebase. However, it is advisable to manually review the tokio changelog for any breaking changes between versions 1.38.0 and 1.39.3 that might affect asynchronous operations or stream handling in your project.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify compatibility of `tokio` version `1.39.3` with other dependencies.

# Test: Search for `tokio` usage in the codebase to identify potential compatibility issues.
rg --type rust 'tokio' -A 5

Length of output: 7469


28-28: Review the impact of updating serde_json to version 1.0.125.

The update from serde_json version 1.0.120 to 1.0.125 may include performance improvements or bug fixes. Verify that the new version does not introduce any breaking changes in JSON serialization/deserialization logic.

Verification successful

No direct usage of serde_json found in the codebase.

The serde_json dependency is listed in Cargo.toml, but no direct usage was detected in the Rust codebase. This suggests that the update to version 1.0.125 should not introduce breaking changes. However, if serde_json is used indirectly, further manual verification might be necessary.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify compatibility of `serde_json` version `1.0.125` with existing JSON handling code.

# Test: Search for `serde_json` usage in the codebase to identify potential compatibility issues.
rg --type rust 'serde_json' -A 5

Length of output: 32


Script:

#!/bin/bash
# Description: Perform a comprehensive search for `serde_json` usage in the Rust codebase to verify compatibility.

# Use `ast-grep` to find all instances where `serde_json` is used in the codebase.
ast-grep --lang rust --pattern 'serde_json::$_'

Length of output: 49


Script:

#!/bin/bash
# Description: Check for `serde_json` in Cargo.toml files and search for alternative usage patterns in the Rust codebase.

# Search for `serde_json` in all Cargo.toml files to confirm its declaration as a dependency.
rg 'serde_json' --glob '**/Cargo.toml'

# Search for potential alternative usage patterns or indirect usage in the codebase.
rg --type rust 'use serde_json' -A 5

Length of output: 147

coderabbitai[bot]
coderabbitai bot previously approved these changes Aug 20, 2024
datelier
datelier previously approved these changes Aug 21, 2024
@kmrmt kmrmt requested review from hlts2 and removed request for kpango August 22, 2024 05:10
Signed-off-by: Kosuke Morimoto <[email protected]>
@kmrmt kmrmt dismissed stale reviews from datelier and coderabbitai[bot] via 42f548b August 22, 2024 07:59
@kmrmt kmrmt force-pushed the feature/agent/rust/modify-directory-structure branch from 833548a to 42f548b Compare August 22, 2024 07:59
@github-actions github-actions bot added size/L and removed size/L labels Aug 22, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 833548a and 42f548b.

Files ignored due to path filters (1)
  • rust/Cargo.lock is excluded by !**/*.lock
Files selected for processing (11)
  • rust/Cargo.toml (1 hunks)
  • rust/bin/agent/Cargo.toml (1 hunks)
  • rust/libs/algorithm/Cargo.toml (1 hunks)
  • rust/libs/algorithm/src/lib.rs (1 hunks)
  • rust/libs/algorithms/faiss/Cargo.toml (1 hunks)
  • rust/libs/algorithms/faiss/src/lib.rs (1 hunks)
  • rust/libs/algorithms/ngt/Cargo.toml (1 hunks)
  • rust/libs/algorithms/ngt/src/input.cpp (1 hunks)
  • rust/libs/algorithms/ngt/src/lib.rs (1 hunks)
  • rust/libs/observability/Cargo.toml (1 hunks)
  • rust/libs/proto/Cargo.toml (1 hunks)
Files skipped from review due to trivial changes (4)
  • rust/libs/algorithm/Cargo.toml
  • rust/libs/algorithms/faiss/Cargo.toml
  • rust/libs/algorithms/ngt/src/input.cpp
  • rust/libs/proto/Cargo.toml
Files skipped from review as they are similar to previous changes (7)
  • rust/Cargo.toml
  • rust/bin/agent/Cargo.toml
  • rust/libs/algorithm/src/lib.rs
  • rust/libs/algorithms/faiss/src/lib.rs
  • rust/libs/algorithms/ngt/Cargo.toml
  • rust/libs/algorithms/ngt/src/lib.rs
  • rust/libs/observability/Cargo.toml

coderabbitai[bot]
coderabbitai bot previously approved these changes Aug 22, 2024
Signed-off-by: Kosuke Morimoto <[email protected]>
@github-actions github-actions bot added size/L and removed size/L labels Aug 22, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 42f548b and 4202915.

Files selected for processing (4)
  • rust/libs/algorithm/Cargo.toml (1 hunks)
  • rust/libs/algorithm/src/lib.rs (1 hunks)
  • rust/libs/algorithms/faiss/Cargo.toml (1 hunks)
  • rust/libs/algorithms/faiss/src/lib.rs (1 hunks)
Files skipped from review due to trivial changes (1)
  • rust/libs/algorithms/faiss/Cargo.toml
Files skipped from review as they are similar to previous changes (3)
  • rust/libs/algorithm/Cargo.toml
  • rust/libs/algorithm/src/lib.rs
  • rust/libs/algorithms/faiss/src/lib.rs

Copy link
Collaborator

@hlts2 hlts2 left a comment

Choose a reason for hiding this comment

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

LGTM!

@kpango kpango merged commit b17cc73 into main Aug 30, 2024
19 of 24 checks passed
@kpango kpango deleted the feature/agent/rust/modify-directory-structure branch August 30, 2024 23:35
vdaas-ci pushed a commit that referenced this pull request Aug 31, 2024
* modify rust package structure

Signed-off-by: Kosuke Morimoto <[email protected]>

* format

Signed-off-by: Kosuke Morimoto <[email protected]>

---------

Signed-off-by: Kosuke Morimoto <[email protected]>
Co-authored-by: Yusuke Kato <[email protected]>
kmrmt added a commit that referenced this pull request Sep 10, 2024
…2590)

* modify rust package structure (#2586)

* modify rust package structure

Signed-off-by: Kosuke Morimoto <[email protected]>

* format

Signed-off-by: Kosuke Morimoto <[email protected]>

---------

Signed-off-by: Kosuke Morimoto <[email protected]>
Co-authored-by: Yusuke Kato <[email protected]>

* resolve conflict

Signed-off-by: Kosuke Morimoto <[email protected]>

---------

Signed-off-by: Kosuke Morimoto <[email protected]>
Co-authored-by: Kosuke Morimoto <[email protected]>
Co-authored-by: Yusuke Kato <[email protected]>
@kpango kpango mentioned this pull request Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants