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

Maybe move default storage dir outside nethermind repo folder? #7795

Open
obasekiosa opened this issue Nov 22, 2024 · 9 comments · May be fixed by #7815
Open

Maybe move default storage dir outside nethermind repo folder? #7795

obasekiosa opened this issue Nov 22, 2024 · 9 comments · May be fixed by #7815

Comments

@obasekiosa
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Deleting an entire repo implies deleting all storage data when nethermind client is ran from source.

Describe the solution you'd like
maybe have client use the HOME fould of XDG* paths for creating these directories.

Describe alternatives you've considered
Just let it be. it shouldn't be an issue.
allows running multiple versions of the code without worrying about data collision.

Additional context
None.

@Dhir0808
Copy link

Hi @obasekiosa , I suggest changing the default storage directory to use the HOME directory or XDG paths (e.g., $XDG_DATA_HOME, $XDG_CONFIG_HOME). This would prevent data loss when deleting or updating the repo and avoid conflicts when running multiple client versions. It would also give users more control over where the data is stored.

I would like to take this issue.

@obasekiosa
Copy link
Contributor Author

@Dhir0808
Sure.
it'd be nice if you start a conversation about it first

I'd just link a couple of people.
@MarekM25
@LukaszRozmej @damian-orzechowski @rubo

@obasekiosa obasekiosa assigned obasekiosa and Dhir0808 and unassigned obasekiosa Nov 25, 2024
@Dhir0808
Copy link

Hi so what I propose is
first we need to define the standard paths following XDGs specs then provide configuration options for custom paths and ensure a backward compatability

so we can create a new class to handle the storage paths
then update the db config to use new path and
then add migration for the existing data
also we will need to update the startup config to handle new paths

@obasekiosa
Copy link
Contributor Author

Hi so what I propose is first we need to define the standard paths following XDGs specs then provide configuration options for custom paths and ensure a backward compatability

so we can create a new class to handle the storage paths then update the db config to use new path and then add migration for the existing data also we will need to update the startup config to handle new paths

@Dhir0808
The goal is to change the default location.(i.e the location that is used if none is specified.
basically change the location of the storage path to be that specified by the os based on the env variable XDG*

pick the approprained XDG* variable and reasonable fallbacks should they be empty.
also consider how the changes would affect release mode/binary.

@Dhir0808
Copy link

@obasekiosa I have tried to move the folder out can you have a look at it and suggest any changes,
it has given me an auth error when I tried to run the app but that maybe because I dont have access to DB ig.

@rubo
Copy link
Contributor

rubo commented Nov 27, 2024

I have mixed feelings about this. While it may make sense in certain situations, it doesn't address a common issue. Previously, we had the NETHERMIND_DATA_DIR environment variable for this very reason, but it didn't get much use across the team, so we removed it.
This said, I don't consider frequently checking out and deleting the repo a typical use case. Usually, when running Nethermind, one should explicitly specify the data directory pointing to a dedicated storage place. That should resolve all the problems.

@obasekiosa
Copy link
Contributor Author

@obasekiosa I have tried to move the folder out can you have a look at it and suggest any changes, it has given me an auth error when I tried to run the app but that maybe because I dont have access to DB ig.

@Dhir0808 this is actually quite good.
But a bit overkill.

What we would have wanted was to simply get the env variable and recursive fallbacks if any are empty and then set the
config baseDb path to the first non empty value.

Also we could do away with the "Nethermind" prefix when naming classes.

But given @rubo concerns which i believe are valid.

A better approach would be to just log a warning on startup that the default directory is being used [and would be lost on deleting the repo (might not make sense when using a compiled binary)].

what do you think @rubo @Dhir0808

@Dhir0808
Copy link

@rubo @obasekiosa Thank you for the feedback! I will start to work on logging a warning on startup which is I feel a good solution to the problem.

@rubo
Copy link
Contributor

rubo commented Nov 28, 2024

The priority is end-user experience, not the dev team. While the data directory should always be explicitly set, and showing a warning makes sense when it's not, I'd like to know the team's opinion about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants