feat: node data location settings#3068
Conversation
shanimal08
changed the title
shanimal08
force-pushed
the
feat/node_dir_ui
branch
from
027ff9e to
7f6bcc8
Compare
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces functionality to allow users to set a custom location for node data. The changes involve updating dependency files, adding a new Tauri command in the backend to handle directory changes, and implementing corresponding frontend UI components and logic. The core backend logic for moving data directories appears to be inverted, which could lead to data loss. Frontend changes correctly integrate with the new backend command and provide a user interface for this feature.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature allowing users to set a custom location for the node data. The changes span both the backend and frontend, including a new Tauri command to handle the directory change, updates to configuration management, and a new UI section in the settings. While the overall implementation is solid, there are a few areas that could be improved, particularly concerning the robustness of file operations and error handling on both the backend and frontend.
shanimal08
changed the title
brianp
left a comment
brianp
left a comment
There was a problem hiding this comment.
A couple things to check up on. Also my build was missing translations:
src-tauri/src/commands.rs
Outdated
| Ok(previous) => { | ||
| let prev_dir_full = previous.join("node"); | ||
| if prev_dir_full.exists() { | ||
| fs::rename(prev_dir_full, new_dir_full) |
There was a problem hiding this comment.
We need a check to ensure the new directory path is completely valid. If it doesn't already exist we could
a) create it
b) fail, because they chose an invalid path
I realize the picker makes it hard to pick non-existent locations, but the API simply accepts data. If a request is made around our picker, what happens?
| let health_timeout = self.health_timeout; | ||
|
|
||
| let mut data_dir_path = base_path.clone(); | ||
| if self.adapter.name() == "local_minotari_node" { |
There was a problem hiding this comment.
I know this isn't your choice but I've decided I hate the identifiers name is a string
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature allowing users to set a custom data directory for the base node. The changes are well-structured, spanning both the frontend and backend to provide the UI, handle the data migration, and update the necessary configurations. The implementation is solid, but I've identified a couple of areas for improvement regarding error handling to make the feature more robust and user-friendly. My specific comments provide suggestions to address these points.
brianp
left a comment
brianp
left a comment
There was a problem hiding this comment.
I'll test it on windows before we merge.
| use tauri::{AppHandle, Manager}; | ||
| use tauri_plugin_cli::CliExt; | ||
|
|
||
| pub async fn check_data_import(app_handle: AppHandle) -> Result<(), anyhow::Error> { |
|
Dunce has fixed the issue on windows. It now appears to be working well 👍🏻 I'm going to let it get a full chain sync and then move dirs and see what the speed is like on windows. |
This reverts commit 4cbb638.
- dialog state after successful move - folder remnants leftover on failed move - moving network specific folder only
Description --- - add a new settings option under `Connections` for setting a custom location to store the base node data - will only show if you're not using `Remote` node - updated config core to include the default location as the initial one - updated the node setup phase and process watcher to use the custom location - added https://docs.rs/fs-more/0.8.1/fs_more/index.html Motivation and Context --- - tari-project#3057 How Has This Been Tested? --- - locally: ~~https://github.com/user-attachments/assets/7ec809b3-0904-4567-8f8e-2b3ee7877ff7~~ (old vid) **new video:** https://github.com/user-attachments/assets/bcc215c3-d762-484e-984a-8aef9a4bb75b **new video with confirm:** https://github.com/user-attachments/assets/8ab21ff6-3b5d-462f-b6d0-326d262277c2 --------- Co-authored-by: brianp <brian.o.pearce@gmail.com>
The custom node data location feature in PR tari-project#3068 failed on macOS with SMB-mounted NAS (and in general on any network filesystem) because fs_more's move_directory can't complete reliably on remote filesystems and the node's LMDB database requires local-disk semantics (atomic renames, directory fsync, extended metadata). Previously a network destination was caught only by the underlying fs_more error, after the node phase had already been shut down and the config updated — leaving users with a cryptic error and a wallet in an inconsistent state. This change: - Adds `detect_network_filesystem(path)` which uses sysinfo::Disks (already a dependency) to find the longest-matching mount point for the target path and checks its filesystem type against a list of known remote filesystems (smbfs, cifs, nfs, nfs4, afpfs, webdav, sshfs, 9p, ceph, glusterfs, beegfs, lustre, etc.). - Runs the check at the very top of `update_data_location`, before any phase shutdown or config mutation, and returns a clear actionable InvokeError explaining that a local disk is required. Closes tari-project#3178 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
Description
Connectionsfor setting a custom location to store the base node dataRemotenodeMotivation and Context
How Has This Been Tested?
https://github.com/user-attachments/assets/7ec809b3-0904-4567-8f8e-2b3ee7877ff7(old vid)new video:
Screen.Recording.2026-02-05.at.15.28.21.mov
new video with confirm:
Screen.Recording.2026-02-06.at.11.42.41.mov