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.

Sign up for GitHub

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

Jump to bottom

Modify the Node structure #401

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Modify the Node structure #401

wants to merge 1 commit into from
+1,072 −269

Conversation

kimhanbeom
Copy link
Contributor

@kimhanbeom kimhanbeom commented Jan 23, 2025 •
edited
Loading

  • Modify the Node structure to allow you to manage the configuration of all remote servers that communicate with the REview.
    • Introduces Remote, a new structure that stores configuration information for remote servers. The data in Remote is stored via Table<Remote>.
    • Add a remotes field of type Vec<Remote> within the Node structure to store the all remote server configuration.
    • Add a Datalake, TiContainer field inside node::kind.
    • Modify migration function migrate_0_29_node.
    • Add migration functionmigrate_0_35_node.

Close: #399

Migration Test

  • Insert a node with version "0.27.1".
    image
  • Migrating the database to version "0.35.0-alpha.1". (using minireview)
    image
  • Migration result.
    • nodeList query
      image
    • query result
      image

@kimhanbeom kimhanbeom changed the title Modify the node structure. Modify the node structure Jan 23, 2025
@codecov Codecov
Copy link

codecov bot commented Jan 23, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 94.32314% with 39 lines in your changes missing coverage. Please review.

Project coverage is 68.81%. Comparing base (3bf9df4) to head (f8fdb79).
Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
src/tables/node.rs 90.63% 25 Missing ⚠️
src/migration.rs 93.45% 14 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #401      +/-   ##
==========================================
+ Coverage   68.19%   68.81%   +0.61%     
==========================================
  Files          95       96       +1     
  Lines       20320    20871     +551     
==========================================
+ Hits        13857    14362     +505     
- Misses       6463     6509      +46

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

sehkone
sehkone reviewed Jan 24, 2025
View reviewed changes
src/tables/node.rs Outdated
@@ -14,7 +14,7 @@ use crate::{
};

#[derive(Default, Debug, Clone, Serialize, Deserialize, PartialEq)]
pub struct Giganto {
pub struct ConnectionlessAgent {
Copy link
Contributor

Choose a reason for hiding this comment

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

While Agent has a key for each instance, ConnectionlessAgent does not. Additionally, Agents are represented as a Vec<Agent>, whereas ConnectionlessAgents are instantiated individually, like giganto and ti_container. Furthermore, ConnectionlessAgent includes its status through the AgentStatus field. These differences highlight inconsistencies, and on top of that, the name ConnectionlessAgent doesn't seem ideal to convey the true meaning. I believe we need a better approach to address these inconsistencies and clarify the concept.

Copy link
Contributor

Choose a reason for hiding this comment

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

@sophie-cluml @kimhanbeom

As we discussed, it would be nice to introduce the Remote struct to handle remote servers that do not use QUIC communication. It will be similar to Agent in many ways.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll proceed with the modifications in that direction.

@kimhanbeom kimhanbeom mentioned this pull request Feb 19, 2025
Open
@kimhanbeom
Modify the Node structure
f8fdb79 
- Modify the `Node` structure to allow you to manage the configuration
  of all remote servers that communicate with the REview.
  - Introduces `Remote`, a new structure that stores configuration
    information for remote servers. The data in `Remote` is stored via
    `Table<Remote>`.
  - Add a `remotes` field of type `Vec<Remote>` within the `Node`
    structure to store the all remote server configuration.
  - Add a `Datalake`, `TiContainer` field inside `node::kind`.
  - Modify migration function `migrate_0_29_node`.
  - Add migration function`migrate_0_35_node`.

Close: petabi#399
@kimhanbeom kimhanbeom changed the title Modify the node structure Modify the Node structure Feb 19, 2025
@kimhanbeom
Copy link
Contributor Author

kimhanbeom commented Feb 19, 2025 •
edited
Loading

@sehkone
I have completed the code modification to handle remote server configuration via the Remote structure.

@kimhanbeom kimhanbeom requested a review from sehkone February 19, 2025 05:02
@sehkone
Copy link
Contributor

sehkone commented Feb 19, 2025

@sehkone I have completed the code modification to handle remote server configuration via the Remote structure.

Thank you for what you've done. I will review the code soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sehkone sehkone Awaiting requested review from sehkone

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Modify the Node structure so as to configure all remote server
2 participants
@kimhanbeom @sehkone