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

Changed default database creation behavior #78

Merged
merged 2 commits into from
Dec 20, 2023
Merged

Conversation

Hydrocharged
Copy link
Collaborator

Previously, when running doltgres, it would look at the current directory to determine if it was a valid DoltgreSQL directory. If it wasn't, then it would attempt to create a doltgres database within the directory. This behavior was chosen since "it just works", which is a selling point of the product. This was causing issues though, as users would run the tool from within the same directory that they downloaded it in. This caused an error message to appear stating that the file was conflicting with the attempt to create the database, however users were still confused by the behavior even with the error message.

This default behavior has been changed. We now use the ~/doltgres/databases directory as the default directory. It no longer matters where you run doltgres from, it will always point to that directory. This directory can be changed by changing the environment variable DOLTGRES_DATA_DIR to a different directory.

In addition, there's another environment variable named DOLTGRES_DATA_DIR_CWD. When this is set to true, then it causes DoltgreSQL to operate with the previous behavior, in that it will use the current directory rather than the global data directory.

Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

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

LGTM, main comments:

  1. One env var is better than two, just allow . and everything works
  2. --data-dir needs to take precedent if it's provided, don't see any tests of that

main.go Outdated Show resolved Hide resolved
testing/bats/setup/query-server-common.bash Show resolved Hide resolved
@Hydrocharged Hydrocharged merged commit 94859da into main Dec 20, 2023
7 checks passed
@Hydrocharged Hydrocharged deleted the daylon/init-changes branch December 20, 2023 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants